Skip to content
Snippets Groups Projects

vkd3d-shader/hlsl: Add radians() and degrees() functions.

Merged Akihiro Sagawa requested to merge sgwaki/vkd3d:wip.angle_units into master
1 unresolved thread

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
3341 3341 return !!add_pow_expr(ctx, params->instrs, params->args[0], params->args[1], loc);
3342 3342 }
3343 3343
3344 static bool intrinsic_radians(struct hlsl_ctx *ctx,
3345 const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
3346 {
3347 struct hlsl_ir_node *arg, *rad;
3348
3349 if (!(arg = intrinsic_float_convert_arg(ctx, params, params->args[0], loc)))
3350 return false;
3351
3352 /* 1 degree = pi/180 rad = 0.0174532925f rad */
3353 if (!(rad = hlsl_new_float_constant(ctx, 0.0174532925f, loc)))
  • When creating a node using a hlsl_new_*() function you also have to add it to the instruction block using hlsl_block_add_instr(), as you can see in other uses of hlsl_new_float_constant() in the same file.

    More complex functions such as intrinsic_float_convert_arg() and those that start with add_ do it themselves, so they don't require to call hlsl_block_add_instr() afterwards.

    Edited by Francisco Casas
  • I wonder if it was copied from log/log10. I see I missed add_insrt() in these two. We probably could have an add_float_constant() helper.

  • Oh, good catch! log/log10 also need this call to hlsl_block_add_instr(). I think it is just serendipity that compilation works without these nodes getting inserted in the instruction block.

    You can improve this MR with a commit adding those missing calls for the log/log10 functions. I don't think that an add_float_constant() helper is necessary though.

  • Oh sorry @nsivov, I thought that I was replying to @sgwaki. The similarity in the user avatars got me confused.

  • I submitted a fix for log() !476 (merged).

  • Author Contributor

    Thanks for comments. Yes, they're surely based on log/log10 functions. I'll add hlsl_block_add_instr() where needed.

  • Honestly, in general I think we can move towards replacing hlsl_new_* with hlsl_add_*. In the vast majority of cases (parsing and lowering passes) we're adding to an existing block, and we really want to lean in that direction anyway so we're not so much using list functions directly.

  • Please register or sign in to reply
  • Francisco Casas
  • Francisco Casas
  • I am ready to approve once my comments have been addressed. Besides those, the implementation seems correct to me. Thanks!

  • Akihiro Sagawa resolved all threads

    resolved all threads

  • Akihiro Sagawa added 2 commits

    added 2 commits

    • b4cc7787 - vkd3d-shader/hlsl: Add radians() function.
    • 7b369b5b - vkd3d-shader/hlsl: Add degrees() function.

    Compare with previous version

  • Akihiro Sagawa added 15 commits

    added 15 commits

    Compare with previous version

  • Francisco Casas approved this merge request

    approved this merge request

  • Akihiro Sagawa added 1 commit

    added 1 commit

    • 43e73f4d - vkd3d-shader/hlsl: Add degrees() function.

    Compare with previous version

  • Author Contributor

    I forgot f suffix in the degrees() patch. Fixed.

  • Giovanni Mascellani approved this merge request

    approved this merge request

  • Elizabeth Figura approved this merge request

    approved this merge request

  • This is fine, but introduces new features and so may be deferred until after 1.10 at this point.

  • This is fine, but introduces new features and so may be deferred until after 1.10 at this point.

    In principle, but I'm still willing to risk it.

  • Henri Verbeet approved this merge request

    approved this merge request

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Please register or sign in to reply
    Loading