From: Rune P. <ru...@me...> - 2006-08-27 18:08:08
|
Hi, Quake3 causes fallback because r300_translate_vertex_shader() returns early and doesn't translate the shader. The culprit: if (!mesa_vp->Base.String) return; To me it looks suspect because checking a pointer to the shader string to verify that the parsed shader is valid doesn't make sense to me. And this check i omitted for fragment translation which uses the same structure. Anything obvious I missed? Rune Petersen |
From: Jerome G. <j.g...@gm...> - 2006-08-27 18:22:47
|
On 8/27/06, Rune Petersen <ru...@me...> wrote: > Hi, > > Quake3 causes fallback because r300_translate_vertex_shader() returns > early and doesn't translate the shader. > > The culprit: > if (!mesa_vp->Base.String) > return; > > To me it looks suspect because checking a pointer to the shader string > to verify that the parsed shader is valid doesn't make sense to me. > And this check i omitted for fragment translation which uses the same > structure. > > Anything obvious I missed? > > > Rune Petersen > I think this check was added few day ago because of a bug spotted in r200 something about a call to fragment shader before any fragment shader program is supplied. Maybe digging a bit the spec and see what to do when called without any program bound. best, Jerome Glisse |
From: Roland S. <rsc...@hi...> - 2006-08-27 18:34:36
|
Rune Petersen wrote: > Hi, > > Quake3 causes fallback because r300_translate_vertex_shader() returns > early and doesn't translate the shader. > > The culprit: > if (!mesa_vp->Base.String) > return; > > To me it looks suspect because checking a pointer to the shader string > to verify that the parsed shader is valid doesn't make sense to me. > And this check i omitted for fragment translation which uses the same > structure. > > Anything obvious I missed? That check is there to check if a shader string was even specified in the first place (see the thread about "Radeon 9200 GetProgramiv(GL_VERTEX_PROGRAM_ARB,..." at the mesa3d-dev list). Maybe there is some trouble with that, in the case of quake3 and r300 I'd suspect it's because no string exists for the shader at all, because quake3 obviously doesn't use vertex shaders and it's internally generated by fixed function to shader conversion code. Roland |
From: Rune P. <ru...@me...> - 2006-08-27 18:57:30
|
Roland Scheidegger wrote: > Rune Petersen wrote: >> Hi, >> >> Quake3 causes fallback because r300_translate_vertex_shader() returns >> early and doesn't translate the shader. >> >> The culprit: >> if (!mesa_vp->Base.String) >> return; >> >> To me it looks suspect because checking a pointer to the shader string >> to verify that the parsed shader is valid doesn't make sense to me. >> And this check i omitted for fragment translation which uses the same >> structure. >> >> Anything obvious I missed? > That check is there to check if a shader string was even specified in > the first place (see the thread about "Radeon 9200 > GetProgramiv(GL_VERTEX_PROGRAM_ARB,..." at the mesa3d-dev list). Maybe > there is some trouble with that, in the case of quake3 and r300 I'd > suspect it's because no string exists for the shader at all, because > quake3 obviously doesn't use vertex shaders and it's internally > generated by fixed function to shader conversion code. > That is what I suspected. The way I see it Base.String is always set along side the Base.Instructions pointer. Since the the code actual depends on Base.Instructions it makes more sense to check it. The only problem is I am unable to test if it failes as intended. Rune Petersen |
From: Brian P. <bri...@tu...> - 2006-08-28 14:52:56
|
Rune Petersen wrote: > Roland Scheidegger wrote: > >>Rune Petersen wrote: >> >>>Hi, >>> >>>Quake3 causes fallback because r300_translate_vertex_shader() returns >>>early and doesn't translate the shader. >>> >>>The culprit: >>>if (!mesa_vp->Base.String) >>> return; >>> >>>To me it looks suspect because checking a pointer to the shader string >>>to verify that the parsed shader is valid doesn't make sense to me. >>>And this check i omitted for fragment translation which uses the same >>>structure. >>> >>>Anything obvious I missed? >> >>That check is there to check if a shader string was even specified in >>the first place (see the thread about "Radeon 9200 >>GetProgramiv(GL_VERTEX_PROGRAM_ARB,..." at the mesa3d-dev list). Maybe >>there is some trouble with that, in the case of quake3 and r300 I'd >>suspect it's because no string exists for the shader at all, because >>quake3 obviously doesn't use vertex shaders and it's internally >>generated by fixed function to shader conversion code. >> > > That is what I suspected. > The way I see it Base.String is always set along side the > Base.Instructions pointer. Since the the code actual depends on > Base.Instructions it makes more sense to check it. > > The only problem is I am unable to test if it failes as intended. If you remove the test for mesa_vp->Base.String does Quake3 work? -Brian |
From: Rune P. <ru...@me...> - 2006-08-28 15:44:00
|
Brian Paul wrote: > Rune Petersen wrote: >> Roland Scheidegger wrote: >> >>> Rune Petersen wrote: >>> >>>> Hi, >>>> >>>> Quake3 causes fallback because r300_translate_vertex_shader() returns >>>> early and doesn't translate the shader. >>>> >>>> The culprit: >>>> if (!mesa_vp->Base.String) >>>> return; >>>> >>>> To me it looks suspect because checking a pointer to the shader string >>>> to verify that the parsed shader is valid doesn't make sense to me. >>>> And this check i omitted for fragment translation which uses the same >>>> structure. >>>> >>>> Anything obvious I missed? >>> >>> That check is there to check if a shader string was even specified in >>> the first place (see the thread about "Radeon 9200 >>> GetProgramiv(GL_VERTEX_PROGRAM_ARB,..." at the mesa3d-dev list). Maybe >>> there is some trouble with that, in the case of quake3 and r300 I'd >>> suspect it's because no string exists for the shader at all, because >>> quake3 obviously doesn't use vertex shaders and it's internally >>> generated by fixed function to shader conversion code. >>> >> >> That is what I suspected. >> The way I see it Base.String is always set along side the >> Base.Instructions pointer. Since the the code actual depends on >> Base.Instructions it makes more sense to check it. >> >> The only problem is I am unable to test if it failes as intended. > > If you remove the test for mesa_vp->Base.String does Quake3 work? It does. The Mesa generated shaders doesn't set Base.String making Base.String a bad choice for checking the validity of the Base struct. I am just trying to find a check that is valid in all cases and can be used by r300 & r200. Rune Petersen |
From: Brian P. <bri...@tu...> - 2006-08-28 16:09:22
|
Rune Petersen wrote: > Brian Paul wrote: > >>Rune Petersen wrote: >> >>>Roland Scheidegger wrote: >>> >>> >>>>Rune Petersen wrote: >>>> >>>> >>>>>Hi, >>>>> >>>>>Quake3 causes fallback because r300_translate_vertex_shader() returns >>>>>early and doesn't translate the shader. >>>>> >>>>>The culprit: >>>>>if (!mesa_vp->Base.String) >>>>> return; >>>>> >>>>>To me it looks suspect because checking a pointer to the shader string >>>>>to verify that the parsed shader is valid doesn't make sense to me. >>>>>And this check i omitted for fragment translation which uses the same >>>>>structure. >>>>> >>>>>Anything obvious I missed? >>>> >>>>That check is there to check if a shader string was even specified in >>>>the first place (see the thread about "Radeon 9200 >>>>GetProgramiv(GL_VERTEX_PROGRAM_ARB,..." at the mesa3d-dev list). Maybe >>>>there is some trouble with that, in the case of quake3 and r300 I'd >>>>suspect it's because no string exists for the shader at all, because >>>>quake3 obviously doesn't use vertex shaders and it's internally >>>>generated by fixed function to shader conversion code. >>>> >>> >>>That is what I suspected. >>>The way I see it Base.String is always set along side the >>>Base.Instructions pointer. Since the the code actual depends on >>>Base.Instructions it makes more sense to check it. >>> >>>The only problem is I am unable to test if it failes as intended. >> >>If you remove the test for mesa_vp->Base.String does Quake3 work? > > > It does. > The Mesa generated shaders doesn't set Base.String making Base.String a > bad choice for checking the validity of the Base struct. > > I am just trying to find a check that is valid in all cases and can be > used by r300 & r200. Does replacing the above test with this work? if (mesa_vp->Base.NumInstructions == 0) return GL_FALSE; -Brian |
From: Rune P. <ru...@me...> - 2006-08-28 16:40:43
|
Brian Paul wrote: > Rune Petersen wrote: >> Brian Paul wrote: >> >>> Rune Petersen wrote: >>> >>>> Roland Scheidegger wrote: >>>> >>>> >>>>> Rune Petersen wrote: >>>>> >>>>> >>>>>> Hi, >>>>>> >>>>>> Quake3 causes fallback because r300_translate_vertex_shader() returns >>>>>> early and doesn't translate the shader. >>>>>> >>>>>> The culprit: >>>>>> if (!mesa_vp->Base.String) >>>>>> return; >>>>>> >>>>>> To me it looks suspect because checking a pointer to the shader >>>>>> string >>>>>> to verify that the parsed shader is valid doesn't make sense to me. >>>>>> And this check i omitted for fragment translation which uses the same >>>>>> structure. >>>>>> >>>>>> Anything obvious I missed? >>>>> >>>>> That check is there to check if a shader string was even specified in >>>>> the first place (see the thread about "Radeon 9200 >>>>> GetProgramiv(GL_VERTEX_PROGRAM_ARB,..." at the mesa3d-dev list). Maybe >>>>> there is some trouble with that, in the case of quake3 and r300 I'd >>>>> suspect it's because no string exists for the shader at all, because >>>>> quake3 obviously doesn't use vertex shaders and it's internally >>>>> generated by fixed function to shader conversion code. >>>>> >>>> >>>> That is what I suspected. >>>> The way I see it Base.String is always set along side the >>>> Base.Instructions pointer. Since the the code actual depends on >>>> Base.Instructions it makes more sense to check it. >>>> >>>> The only problem is I am unable to test if it failes as intended. >>> >>> If you remove the test for mesa_vp->Base.String does Quake3 work? >> >> >> It does. >> The Mesa generated shaders doesn't set Base.String making Base.String a >> bad choice for checking the validity of the Base struct. >> >> I am just trying to find a check that is valid in all cases and can be >> used by r300 & r200. > > Does replacing the above test with this work? > > if (mesa_vp->Base.NumInstructions == 0) > return GL_FALSE; yes (remove GL_FALSE, void function) Rune Petersen |
From: Brian P. <bri...@tu...> - 2006-08-28 19:43:24
|
Rune Petersen wrote: > Brian Paul wrote: > >>Rune Petersen wrote: >> >>>Brian Paul wrote: >>> >>> >>>>Rune Petersen wrote: >>>> >>>> >>>>>Roland Scheidegger wrote: >>>>> >>>>> >>>>> >>>>>>Rune Petersen wrote: >>>>>> >>>>>> >>>>>> >>>>>>>Hi, >>>>>>> >>>>>>>Quake3 causes fallback because r300_translate_vertex_shader() returns >>>>>>>early and doesn't translate the shader. >>>>>>> >>>>>>>The culprit: >>>>>>>if (!mesa_vp->Base.String) >>>>>>> return; >>>>>>> >>>>>>>To me it looks suspect because checking a pointer to the shader >>>>>>>string >>>>>>>to verify that the parsed shader is valid doesn't make sense to me. >>>>>>>And this check i omitted for fragment translation which uses the same >>>>>>>structure. >>>>>>> >>>>>>>Anything obvious I missed? >>>>>> >>>>>>That check is there to check if a shader string was even specified in >>>>>>the first place (see the thread about "Radeon 9200 >>>>>>GetProgramiv(GL_VERTEX_PROGRAM_ARB,..." at the mesa3d-dev list). Maybe >>>>>>there is some trouble with that, in the case of quake3 and r300 I'd >>>>>>suspect it's because no string exists for the shader at all, because >>>>>>quake3 obviously doesn't use vertex shaders and it's internally >>>>>>generated by fixed function to shader conversion code. >>>>>> >>>>> >>>>>That is what I suspected. >>>>>The way I see it Base.String is always set along side the >>>>>Base.Instructions pointer. Since the the code actual depends on >>>>>Base.Instructions it makes more sense to check it. >>>>> >>>>>The only problem is I am unable to test if it failes as intended. >>>> >>>>If you remove the test for mesa_vp->Base.String does Quake3 work? >>> >>> >>>It does. >>>The Mesa generated shaders doesn't set Base.String making Base.String a >>>bad choice for checking the validity of the Base struct. >>> >>>I am just trying to find a check that is valid in all cases and can be >>>used by r300 & r200. >> >>Does replacing the above test with this work? >> >> if (mesa_vp->Base.NumInstructions == 0) >> return GL_FALSE; > > > yes (remove GL_FALSE, void function) > OK, I checked in this change. The r200 version needs the return value. -Brian |