Am 29.01.2012 03:02, schrieb Marek Olšák: > On Sat, Jan 28, 2012 at 2:37 AM, Roland Scheidegger <srol...@vmware.com> > wrote: >> Am 28.01.2012 01:38, schrieb Marek Olšák: >>> Hi everyone, >>> >>> the subject says it all. This series fixes gl_PointSize with transform >>> feedback. There is a new piglit test to verify that a driver does clamping >>> properly during rasterization: vs-point_size-zero >>> >>> I haven't changed Draw, because softpipe and llvmpipe do the clamping >>> internally somewhere. (I didn't take a look where they do it, but they pass >>> the test, which can't be said about r600 with point size clamping disabled) >>> >>> The only drivers I am not sure about are i915 and nouveau. >>> >>> Please review. >> >> This looks generally ok to me. I'd like to see more comment for [4/4], >> e.g. why this is ok (something along the lines that drivers are expected >> to clamp against their advertized point size limits whatever they are >> and depending on point smooth etc.). > > More comment where? In the commit message or in the code? Commit messages is just fine.
> >> For [1/4] I can't quite tell off-hand if the point size min rs applies >> everywhere only hope so... >> I believe the conditions in [1,2,3/4] should also take multisampling >> into account, since point rendering with multisampling also works >> differently (and will produce zero-sized points). >> Also maybe for the hw drivers there should be some comment why actually >> min size is 1.0, as this is merely a workaround for chips which can't >> follow legacy OGL's totally silly point rasterization rules (about as >> silly as smooth points...). > > I updated the patches, they can be read here: > http://cgit.freedesktop.org/~mareko/mesa/log/?h=point-size-clamp > > I added a helper function returning the expected minimum point size, > it also checks for multisample and gl_rasterization_rules: > http://cgit.freedesktop.org/~mareko/mesa/commit/?h=point-size-clamp&id=a57d828e924c3fbee1d4707c3cefe4a8b92deef6 Looks good to me. I think the language is somewhat unfortunate in some places ("The point size should be clamped to this value at the rasterizer stage.") because it's not really a size clamp as such but rather a "poorman GL point rasterization implementation" but that's really nitpicking. (OT I'm actually also not sure why gl_Pointsize of 0.0 is deemed illegal by OGL, I can't see why this would be problematic at least for non-legacy rules.) _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev