Skip to content
Snippets Groups Projects

vkd3d-shader/fx: Add an option to include empty buffers in the effect binary.

Merged Nikolay Sivov requested to merge nsivov/vkd3d:empty_cb into master
2 unresolved threads

Signed-off-by: Nikolay Sivov nsivov@codeweavers.com

Merge request reports

Merge request pipeline #24767 skipped

Merge request pipeline skipped for e1e63672

Merged by Alexandre JulliardAlexandre Julliard 1 year ago (Apr 2, 2024 10:23pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
202 203 " targets. Valid values are 'upper-left' (default) and\n"
203 204 " 'lower-left'. The only value supported by Vulkan\n"
204 205 " environments is 'upper-left'.\n"
206 " --include-empty-buffers\n"
207 " Write empty constant buffers descriptions. This option is only meaningful\n"
208 " for fx_4_0 and fx_4_1 profiles.\n"
  • Comment on lines +206 to +208

    Would it make sense to start namespacing options that only apply to some shader types or profiles? For example using --fx-include-empty-buffers or even --fx4-include-empty-buffers. Not necessarily this time, but maybe start thinking about it. Also for HLSL options, for example: --hlsl-profile and --hlsl-semantic-compat-map instead of what we have now.

  • Nikolay Sivov changed this line in version 3 of the diff

    changed this line in version 3 of the diff

  • Please register or sign in to reply
  • Giovanni Mascellani approved this merge request

    approved this merge request

    • Would it make sense to start namespacing options that only apply to some shader types or profiles? For example using --fx-include-empty-buffers or even --fx4-include-empty-buffers. Not necessarily this time, but maybe start thinking about it. Also for HLSL options, for example: --hlsl-profile and --hlsl-semantic-compat-map instead of what we have now.

      Broadly yes, although I don't think we should just remove the existing options; at best we could deprecate them and remove them from --help.

      +        "  --include-empty-buffers\n"
      +        "                        Write empty constant buffers descriptions. This option is only meaningful\n"
      +        "                        for fx_4_0 and fx_4_1 profiles.\n"

      That line is a bit long; we try not to go over 80 columns in the --help output. We could potentially do line-breaking at run-time though, that would also allow us to eventually use the actual terminal width, which may very well be a bit wider than 80 columns.

    • Author Developer

      Thanks, adjusted both. I used "fx" prefix, "fx4" seems too specific.

    • Would it make sense to start namespacing options that only apply to some shader types or profiles? For example using --fx-include-empty-buffers or even --fx4-include-empty-buffers. Not necessarily this time, but maybe start thinking about it. Also for HLSL options, for example: --hlsl-profile and --hlsl-semantic-compat-map instead of what we have now.

      Broadly yes, although I don't think we should just remove the existing options; at best we could deprecate them and remove them from --help.

      Sure, it wouldn't be appropriate to break command lines that now work, so the current options could be added as aliases of the prefixed ones.

    • Please register or sign in to reply
  • Nikolay Sivov added 1 commit

    added 1 commit

    • be00b3de - vkd3d-shader/fx: Add an option to include empty buffers in the effect binary.

    Compare with previous version

  • Henri Verbeet approved this merge request

    approved this merge request

  • Alexandre Julliard added 15 commits

    added 15 commits

    • be00b3de...64418dd2 - 13 commits from branch wine:master
    • c509c85f - vkd3d-shader/hlsl: Allow annotations on constant buffers.
    • e1e63672 - vkd3d-shader/fx: Add an option to include empty buffers in the effect binary.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Please register or sign in to reply
    Loading