On 05/17/2018 11:56 PM, Mathias Fröhlich wrote: > Hi Dave, > > On Friday, 18 May 2018 06:57:19 CEST Dave Airlie wrote: >>> May be I should take care of all of these type of asserts, also the ones >>> with MaxVertexAttribRelativeOffset and care for not checking them >>> when the extension version is unavailable or checking against the OpenGL >>> spec guaranteed minimum values for both constants instead of the actual >>> ones. >>> ... means, there are more asserts of this kind. >>> >>> Well, alternatively since you probably aim for supporting GL4.4 at one >>> point, you >>> could alternatively set the constant already? AFAICT the >>> PIPE_CAP_MAX_VERTEX_ATTRIB_STRIDE is only used to set the >>> constant and does not imply anything beyond. >> >> Well it's not just virgl, won't this break things like r300 and i915? > > I dont think so. > > The constant is set to the opengl mandated minimum of 2048 in > _mesa_init_constants which seems not overwritten by any classic driver. > In gallium there is the cap which overwrites the constant past setting > it from mesa. As far as I greped yesterday, only virgil just returns 0. > The rest returns the mandated 2048, there is one exception with etnaviv > that returns 128 when being asked for PIPE_CAP_MAX_VERTEX_ATTRIB_STRIDE. > If you look into etnaviv this stems from having only 8 bits in the hardware > register for the stride.
Or maybe leave the assert but have virgl return a real value instead of 0? The whole reason that _mesa_init_constants sets things to sensible default values is so that we don't need workarounds in the main code for drivers that don't support the functionality. > So, I realize, that I have been taking the constant MaxVertexAttribStride > as either a well maintained hardware limit or at least as a lower bound to > that hardware limit. Means I did not expect this presence of that > value (well being != 0, I mean) to be dependent on the OpenGL version > but on the backing driver. > > What does that mean to virgl. Well, I see you cant get (OpenGL < 4.4 or no > GL_ARB_vertex_attrib_binding) or dont know (virgil does not ask the host > currently) the real value. > Also the current implementation just checks this to give a some hint what goes > wrong, especially there should be no crash if the assert fails. > That together is why I just said 'fine, I have to treat that differently, you > get my r-b > so that my assert gets out of your way'. > > Putting that all together, I see the route: > * just remove the assert, cross you fingers and dont check at all. > Most likely it all works. > All modern hardware can do 2048 and beyond and the vbo module > can only produce strides up to VBO_ATTRIB_MAX*4*4 < 2048. > It wont work if the virtualization host runs etnaviv or equivalent > limited and somebody does a high count of vertex attributes through > virgil - unlikely. > * Extend the assert to > assert(MaxVertexAttribStride == 0 || stride < MaxVertexAttribStride) > Then virgil does not get the check. > * Replace the assert with a gl error. Then the application gets at least > a hint that there was some resource exceeded. You will need to give > some limit for virgl or we play the MaxVertexAttribStride == 0 game > so that virgil just does not get that checked. > > I think I need to downgrade the assert to an error at least as I cannot > internally to mesa guarantee that it has to hold. > > I am more concerned about the MaxVertexAttribRelativeOffset constant > which is required from _mesa_update_vao_derived_arrays. It is again set to > the OpenGL mandated minimum of 2047 from _mesa_init_constants. Since > there is no gallium cap for that, it cannot just be set to 0 from virgil. > Etnaviv is again technically not able to handle the OpenGL mandated > minimum but should again work for most of the practical cases. > But finding a zero in MaxVertexAttribRelativeOffset would not work > for _mesa_update_vao_derived_arrays. > > best > > Mathias > > > _______________________________________________ > 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
