Skip to content
Snippets Groups Projects

vkd3d-shader: Refactor common code between vkd3d_shader_compile() and vkd3d_shader_scan().

Merged Giovanni Mascellani requested to merge giomasce/vkd3d:stromboli into master
2 unresolved threads

Merge request reports

Merge request pipeline #23590 skipped

Merge request pipeline skipped for 172e7edf

Approved by

Merged by Alexandre JulliardAlexandre Julliard 1 year ago (Mar 8, 2024 10:47pm UTC)

Merge details

  • Changes merged into master with 172e7edf.
  • Did not delete the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Giovanni Mascellani approved this merge request

    approved this merge request

    • -            ERR("Unsupported source type %#x.\n", compile_info->source_type);
      -            ret = VKD3D_ERROR_INVALID_ARGUMENT;
      -            break;
      +            vkd3d_unreachable();

      I imagine this is to make vkd3d_shader_scan() more similar to vkd3d_shader_compile(), but I think vkd3d_unreachable() is worse than the existing code.

      I feel a bit ambivalent about this MR. It's nice to get rid of some duplication and room for potential inconsistencies between vkd3d_shader_scan() and vkd3d_shader_compile(), but I don't love scan_or_compile(), and its "compile" argument in particular.

    • I imagine this is to make vkd3d_shader_scan() more similar to vkd3d_shader_compile(), but I think vkd3d_unreachable() is worse than the existing code.

      Even if the source type is already validated by vkd3d_shader_validate_compile_info()? Which use do you see into validating the source type twice?

      I feel a bit ambivalent about this MR. It's nice to get rid of some duplication and room for potential inconsistencies between vkd3d_shader_scan() and vkd3d_shader_compile(), but I don't love scan_or_compile(), and its "compile" argument in particular.

      I can see how that commit can be a bit more controversial than the others. Let's drop it for the moment.

    • Please register or sign in to reply
  • added 26 commits

    • be8a7767...a0d52dc3 - 21 commits from branch wine:master
    • a35f96e2 - vkd3d-shader: Do not validate the source type twice.
    • 7c591864 - vkd3d-shader: Treat the HLSL case specially in vkd3d_shader_scan().
    • 185dce7b - vkd3d-shader: Treat the HLSL case specially in vkd3d_shader_compile().
    • d81b446c - vkd3d-shader: Refactor common code for vkd3d_shader_scan().
    • 423c9f04 - vkd3d-shader: Refactor common code for vkd3d_shader_compile().

    Compare with previous version

    • I imagine this is to make vkd3d_shader_scan() more similar to vkd3d_shader_compile(), but I think vkd3d_unreachable() is worse than the existing code.

      Even if the source type is already validated by vkd3d_shader_validate_compile_info()? Which use do you see into validating the source type twice?

      It's more about disliking vkd3d_unreachable()/abort(). That's a discussion we've had a few times before, of course; what's different this time is that we've seen a number of bugs come in through WineHQ Bugzilla with assertions failing inside vkd3d-shader since the last time we had that discussion.

    • Ok. I don't like it, but here it is.

    • Please register or sign in to reply
  • added 4 commits

    • 9f379197 - vkd3d-shader: Treat the HLSL case specially in vkd3d_shader_scan().
    • e8c71dfa - vkd3d-shader: Treat the HLSL case specially in vkd3d_shader_compile().
    • b411bac4 - vkd3d-shader: Refactor common code for vkd3d_shader_scan().
    • 56c620e4 - vkd3d-shader: Refactor common code for vkd3d_shader_compile().

    Compare with previous version

  • Henri Verbeet approved this merge request

    approved this merge request

  • Alexandre Julliard added 26 commits

    added 26 commits

    • 56c620e4...b940d79a - 22 commits from branch wine:master
    • 3a1c32ac - vkd3d-shader: Treat the HLSL case specially in vkd3d_shader_scan().
    • bbd37a76 - vkd3d-shader: Treat the HLSL case specially in vkd3d_shader_compile().
    • 0e5d6a97 - vkd3d-shader: Refactor common code for vkd3d_shader_scan().
    • 172e7edf - vkd3d-shader: Refactor common code for vkd3d_shader_compile().

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

Please register or sign in to reply
Loading