Re: [PATCH v7 0/4] MR582: vkd3d-dxbc: Add options to fix a DXBC shader checksum.

[Giovanni Mascellani (＠giomasce)]

On Fri Mar  8 21:54:34 2024 +0000, Henri Verbeet wrote:
> > -        if ((option = getopt_long(argc, argv, "htV", long_options,
>
> NULL)) == -1)
> > -
> >    if ((option = getopt_long(argc, argv, "ehtVo:", long_options,
>
> NULL)) == -1)
> >
>
> "o:" belongs in the next commit.
> > +            case 'e':
> > +            case OPTION_EMIT:
> > +                action_push(options, ACTION_TYPE_EMIT);
> > +                if (isatty(fileno(stdout)))
> > +                {
> > +                    fprintf(stderr, "Output is a tty and output
>
> format is binary, exiting.\n"
> > -
> >                        "If this is really what you intended,
>
> specify the output explicitly.\n");
> > -
> >                return false;
> > -
> >            }
> > -
> >            break;
>
> Relatedly, we can't specify the output explicitly yet in this commit.
> > +        "Currently this tool can only re-emit the same DXBC blob it
>
> loaded. However it\n"
> > -
> >    "freshly recompute the checksum while doing so, so --emit can
>
> be useful\n"
> > -
> >    "together with --ignore-checksum to fix the checksum for a blob.\n";
>
> I think the wording is a bit awkward here. I'd propose "However, it will
> recompute ..." or possibly "However, it recomputes ..."
> Note that multiple --emit options will cause vkd3d-dxbc to output the
> DXBC blob multiple times. I think that's fine, perhaps even desirable,
> but it may be worth explicitly pointing out.
> > -                if (!write_output(stdout, &serialized))
> > +                if (!(output = open_output(options.output_filename, &close_output)))
> > +                {
> > +                    vkd3d_shader_free_shader_code(&serialized);
> > +                    goto done;
> > +                }
> > +
> > +                if (!write_output(output, &serialized))
>
> That should use "action->u.output_filename" instead of
> "options.output_filename", right?

Done, together with another couple of minor details I discovered in the meantime.