March 2024 - Wine-GitLab - list.winehq.org

Re: [PATCH v2 0/3] MR5386: user32: Add LoadKeyboardLayoutEx stub. - approved
by Alexandre Julliard (＠julliard) March 22, 2024

March 22, 2024

This merge request was approved by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5386

1 0

Re: [PATCH v2 0/7] MR739: vkd3d-shader/hlsl: Parse state blocks, part 1.
by Francisco Casas (＠fcasas) March 22, 2024

March 22, 2024

On Fri Mar 22 20:59:49 2024 +0000, Nikolay Sivov wrote: > My concern, just by looking at it, is about how hard would it be to > extract relevant cases from instructions. Is it easy to tell already > that we got a constant load, array variable load with constant index, > array variable load with variable index, or more complicated expression? > Those are important to distinguish, because they are encoded differently. well, first we have to find out if native does some compilation passes such as copy-propagation and constant folding to lower complicated expressions into constants, and apply those passes. For instance, are rhs expressions such as `ANISOTROPIC + 1` marked as a of the _constant load_ type in the effects metadata? After applying the passes we have to check the type of the last instruction of the block (or args[0], in case ths is not a complex initializer with braces) which represents the whole expression, and see if it is HLSL_IR_CONSTANT, HLSL_IR_INDEX (in which case we have to check if the index part is constant) or HLSL_IR_EXPR. We should also probably introduce a pass exclusive to fx.c to lower known HLSL_IR_UNDECLARED_LOAD into constants, so constant folding (if required) can work. I don't know yet how complex initializers with braces work in this case, if at all, but they parse. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_65852

1 0

Re: [PATCH v2 0/7] MR739: vkd3d-shader/hlsl: Parse state blocks, part 1.
by Zebediah Figura (＠zfigura) March 22, 2024

March 22, 2024

Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.h: > /* Scope that contains annotations for this variable. */ > struct hlsl_scope *annotations; > > - /* The state block on the variable's declaration, if any. > + /* A list containing the state block on the variable's declaration, if any. > + * An array variable may contain multiple state blocks. > * These are only really used for effect profiles. */ > - struct hlsl_state_block *state_block; > + struct list state_blocks; Do we really want this to be a list? I feel like an array is probably going to be nicer to work with... -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_65851

1 0

Re: [PATCH v2 0/7] MR739: vkd3d-shader/hlsl: Parse state blocks, part 1.
by Zebediah Figura (＠zfigura) March 22, 2024

March 22, 2024

Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.h: > HLSL_IR_STORE, > HLSL_IR_SWIZZLE, > HLSL_IR_SWITCH, > + HLSL_IR_UNDECLARED_LOAD, I don't like this name. It's not a load, since it's not coming from a variable. I would rather say something like "effect constant" or "stateblock constant". -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_65850

1 0

Re: [PATCH v2 0/7] MR739: vkd3d-shader/hlsl: Parse state blocks, part 1.
by Zebediah Figura (＠zfigura) March 22, 2024

March 22, 2024

Zebediah Figura (@zfigura) commented about tests/hlsl/fx-syntax.shader_test: > +float3 vec; > + > +float4 main() : sv_target { return 0; } > + > +DepthStencilState dss1 > +{ > + RandomField = vec.w; > +}; > + > + > +% Test function call syntax for state blocks. Unlike assignment syntax, only these names are allowed. > +% The parameter count is also checked. > +[pixel shader todo] > +sampler sam > +{ > + SetBlendState(foo, bar, baz); // 3 parameters Can you mix function calls and assignments? I'd assume so but it'd be trivial to test. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_65848

1 0

Re: [PATCH v2 0/7] MR739: vkd3d-shader/hlsl: Parse state blocks, part 1.
by Zebediah Figura (＠zfigura) March 22, 2024

March 22, 2024

Zebediah Figura (@zfigura) commented about tests/hlsl/fx-syntax.shader_test: > + pass > + { > + cat = SetPixelShader(foobar); > + } > +} > + > + > +% Test use of a DepthStencilState in SetDepthStencilState(). > +[pixel shader todo] > +DepthStencilState dss1 > +{ > + DepthEnable = false; > + DepthWriteMask = Zero; > + DepthFunc = Less; > + foobar_Field = 22; > +}; This test is a bit weird, because as we've seen, SetDepthStencilState() doesn't even validate anything [unless we're in an effect target, presumably]. Is this actually testing anything? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_65849

1 0

Re: [PATCH v2 0/7] MR739: vkd3d-shader/hlsl: Parse state blocks, part 1.
by Zebediah Figura (＠zfigura) March 22, 2024

March 22, 2024

Zebediah Figura (@zfigura) commented about tests/hlsl/fx-syntax.shader_test: > + } > + }, > + { > + { > + Filter = ANISOTROPIC; > + }, > + { > + Filter = ANISOTROPIC; > + } > + } > +}; > + > +float4 main() : sv_target { return 0; } > + > + > +% Arrays of size 1 can still use a single state block without the need to put it inside a list. There's a lot of tests for valid array usage, but not invalid array usage. What if the count is mismatched? Can an array of size 1 take interior braces? Can a non-array take interior braces? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_65847

1 0

Re: [PATCH v2 0/7] MR739: vkd3d-shader/hlsl: Parse state blocks, part 1.
by Zebediah Figura (＠zfigura) March 22, 2024

March 22, 2024

Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.y: > > state_block: > %empty > - | state_block state > + { > + if (!($$ = hlsl_alloc(ctx, sizeof(*$$)))) > + YYABORT; > + } > + | state_block any_identifier '[' C_INTEGER ']' '=' complex_initializer ';' You could probably avoid the duplication with a separate "%empty | [int]" rule. Or alternatively use "arrays" and add an explicit check for count > 1. Is C_INTEGER correct here, or should we allow compile-time constants? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_65846

1 0

Re: [PATCH v2 0/7] MR739: vkd3d-shader/hlsl: Parse state blocks, part 1.
by Zebediah Figura (＠zfigura) March 22, 2024

March 22, 2024

Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.h: > /* Scope that contains annotations for this variable. */ > struct hlsl_scope *annotations; > > + /* The state block on the variable's declaration, if any. > + * These are only really used for effect profiles. */ > + struct hlsl_state_block *state_block; Does this need to be a pointer? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_65845

1 0

Re: [PATCH v2 0/7] MR739: vkd3d-shader/hlsl: Parse state blocks, part 1.
by Zebediah Figura (＠zfigura) March 22, 2024

March 22, 2024

Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.y: > %type <variable_def> variable_def > %type <variable_def> variable_def_typed > > +%type <state_block> state_block > + This is out of alphabetical order. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/739#note_65843

1 0