How about we remove all opcodes that are unused? Like: SAMPLE_POS SAMPLE_INFO SAMPLE SAMPLE_I SAMPLE_I_MS SAMPLE_B SAMPLE_C SAMPLE_C_LZ SAMPLE_D SAMPLE_L GATHER4 SVIEWINFO SWITCH CASE DEFAULT ENDSWITCH BREAKC BGNSUB ENDSUB CAL
Marek On Tue, Jun 13, 2017 at 4:00 PM, Brian Paul <bri...@vmware.com> wrote: > On 06/12/2017 05:33 PM, Roland Scheidegger wrote: >> >> Am 12.06.2017 um 21:19 schrieb Brian Paul: >>> >>> On 06/12/2017 11:50 AM, Nicolai Hähnle wrote: >>>> >>>> On 12.06.2017 19:37, Brian Paul wrote: >>>>> >>>>> For the SAMPLE_POS and SAMPLE_INFO opcodes, clarify resource vs. render >>>>> target queries, range of postion values, swizzling, etc. We basically >>>>> follow the DX10.1 conventions. >>>>> >>>>> For the TXQS opcode and TGSI_SEMANTIC_SAMPLEID, clarify return value >>>>> and type. >>>>> >>>>> For the TGSI_SEMANTIC_SAMPLEPOS system value, clarify the range of >>>>> positions returned. >>>>> --- >>>>> src/gallium/docs/source/tgsi.rst | 53 >>>>> +++++++++++++++++++++++++++++++--------- >>>>> 1 file changed, 42 insertions(+), 11 deletions(-) >>>>> >>>>> diff --git a/src/gallium/docs/source/tgsi.rst >>>>> b/src/gallium/docs/source/tgsi.rst >>>>> index 7fb963f..310b49d 100644 >>>>> --- a/src/gallium/docs/source/tgsi.rst >>>>> +++ b/src/gallium/docs/source/tgsi.rst >>>>> @@ -982,7 +982,9 @@ XXX doesn't look like most of the opcodes really >>>>> belong here. >>>>> .. opcode:: TXQS - Texture Samples Query >>>>> This retrieves the number of samples in the texture, and stores it >>>>> - into the x component. The other components are undefined. >>>>> + into the x component as an unsigned integer. The other components >>>>> are >>>>> + undefined. If the texture is not multisampled, this function >>>>> returns >>>>> + (1, 0, 0, 0). >>>>> .. math:: >>>>> @@ -2538,14 +2540,40 @@ after lookup. >>>>> .. opcode:: SAMPLE_POS >>>>> - Query the position of a given sample. dst receives float4 (x, y, >>>>> 0, 0) >>>>> - indicated where the sample is located. If the resource is not a >>>>> multi-sample >>>>> - resource and not a render target, the result is 0. >>>>> + Query the position of a sample in the given resource or render >>>>> target >>>>> + when per-sample fragment shading is in effect. >>>>> + >>>>> + Syntax: ``SAMPLE_POS dst, source, sample_index`` >>>>> + >>>>> + dst receives float4 (x, y, 0, 0) indicated where the sample is >>>>> located. >>>>> + Sample locations are in the range [0, 1] where 0.5 is the center of >>>>> the >>>>> + fragment. >>>>> + >>>>> + source is either a sampler view (to indicate a shader resource) or >>>>> temp >>>>> + register (to indicate the render target). The source register may >>>>> have >>>>> + an optional swizzle to apply to the returned result >>>>> + >>>>> + sample_index is an integer scalar indicating which sample position >>>>> is to >>>>> + be queried. >>>>> + >>>>> + If per-sample shading is not in effect or the source resource or >>>>> render >>>>> + target is not multisampled, the result is (0, 0, 0, 0). >>>> >>>> >>>> We don't actually implement SAMPLE_POS in radeonsi, but are you sure >>>> this is a good idea? Wouldn't it make more sense that the result is >>>> (0.5, 0.5, 0, 0)? >>> >>> >>> I think you're right. The DX docs say the default (in some cases) is >>> zeros, which would correspond the fragment center. So that'd be (0.5, >>> 0.5, 0, 0) for GL (which is what the extension spec indicates). >> >> >> I'd argue you strictly can't use these opcodes with GL (but they are >> probably not specified correctly for dx10 neither right now). >> I'm not sure which extension you're talking about, but generally for all >> gallium sample opcodes they won't work in gl in any case because you >> need opcodes taking samplers, not sampler views (we're going to also >> need a dx10 lod opcode version for that reason.) > > > Actually, TGSI_OPCODE_SAMPLE_POS and _INFO aren't even used yet. The state > tracker doesn't emit them and no driver implements them. I was trying to be > proactive and clarify them while working with the MSAA-related semantic > labels. I believe Zack added them originally. Maybe we should just put a > big "XXX do not use" comment on them for the time being. I'm open to > suggestions. > > >> (And yes, d3d10.1 specifies the value in there is relative to pixel >> center, hence 0). > > > I also need to check which way the Y axis is defined. AFAICT, the drivers > which use TGSI_SEMANTIC_SAMPLEPOS assume the positions are in the GL > coordinate system. I suspect that in addition to the 0.5 bias, DX10 also > inverts Y. > > >> Albeit those opcodes working both for rasterizer and resources is still >> weird... > > > That's a D3D thing. Rather than introduce a D3D "rasterizer" register in > gallium I opted to use a temp reg. > > -Brian > > > >> >> Roland >> >> >>> I'll post updated patches... >>> >>> -Brian >>> >>>> >>>> The rest look good to me. >>>> >>>> Cheers, >>>> Nicolai >>>> >>>> >>>>> .. opcode:: SAMPLE_INFO >>>>> - dst receives number of samples in x. If the resource is not a >>>>> multi-sample >>>>> - resource and not a render target, the result is 0. >>>>> + Query the number of samples in a multisampled resource or render >>>>> target. >>>>> + >>>>> + Syntax: ``SAMPLE_INFO dst, source`` >>>>> + >>>>> + dst receives int4 (n, 0, 0, 0) where n is the number of samples in a >>>>> + resource or the render target. >>>>> + >>>>> + source is either a sampler view (to indicate a shader resource) or >>>>> temp >>>>> + register (to indicate the render target). The source register may >>>>> have >>>>> + an optional swizzle to apply to the returned result >>>>> + >>>>> + If per-sample shading is not in effect or the source resource or >>>>> render >>>>> + target is not multisampled, the result is (1, 0, 0, 0). >>>>> .. _resourceopcodes: >>>>> @@ -3284,15 +3312,18 @@ TGSI_SEMANTIC_SAMPLEID >>>>> """""""""""""""""""""" >>>>> For fragment shaders, this semantic label indicates that a system >>>>> value >>>>> -contains the current sample id (i.e. gl_SampleID). >>>>> -This is an integer value, and only the X component is used. >>>>> +contains the current sample id (i.e. gl_SampleID) as an unsigned int. >>>>> +Only the X component is used. If per-sample shading is not enabled, >>>>> +the result is (0, 0, 0, 0). >>>>> TGSI_SEMANTIC_SAMPLEPOS >>>>> """"""""""""""""""""""" >>>>> -For fragment shaders, this semantic label indicates that a system >>>>> value >>>>> -contains the current sample's position (i.e. gl_SamplePosition). Only >>>>> the X >>>>> -and Y values are used. >>>>> +For fragment shaders, this semantic label indicates that a system >>>>> +value contains the current sample's position as float4(x, y, 0, 0) in >>>>> +the render target (i.e. gl_SamplePosition) when per-fragment shading >>>>> +is in effect. Position values are in the range [0, 1] where 0.5 is >>>>> +the center of the fragment. >>>>> TGSI_SEMANTIC_SAMPLEMASK >>>>> """""""""""""""""""""""" >>>>> >>>> >>>> > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev