[Mesa-dev] Revert "gallium: add point size clamp to implementation limits in vertex shader"

[Marek Olšák]

On Mon, Jan 23, 2012 at 6:34 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 23.01.2012 15:44, schrieb Marek Olšák:
>> Hi,
>>
>> I will have to (at least partially) revert this commit, as well as the
>> forked code in glsl_to_tgsi:
>>
>>> commit 2c326e72664e65166c68b027b26aaf373f3be36d
>>> Author: Roland Scheidegger <sroland at vmware.com>
>>> Date:   Thu Feb 4 19:23:09 2010 +0100
>>>
>>>     gallium: add point size clamp to implementation limits in vertex shader
>>>
>>>     The point size min/max registers (unused by mesa state tracker) were removed
>>>     since most hardware couldn't do much with them. However, we don't want to have
>>>     to rely on hw to do point size clamping correctly to implementation
>>>     dependent limits, hence have to do that in the vertex shader. This should also
>>>     solve a potential problem with (non-AA) points smaller than 1.0 which according
>>>     to OGL still have size 1.0.
>>>     Note that OGL point rendering is odd, in particular point sprites are rasterized
>>>     differently to points. Some hardware might support those different modes, but in
>>>     any case the different clamping values used for smooth/multisampled/sprite
>>>     enabled points might help a bit for hw which rasterizes points the same as point
>>>     sprites.
>>>     Also tweak mesa's ff to vertex shader translation so don't have to clamp twice in
>>>     case of point attenuation.
>>
>> I mean the part which rewrites vertex shaders to clamp the point size
>> in st/mesa. We can't do that if transform feedback is enabled. The
>> point size must be unmodified when writing it into a transform
>> feedback buffer. Also it's unclear to me why we clamp the point size
>> at all.
>>
>> If we want to do any clamping, we should do it after transform
>> feedback, that is after the vertex and geometry shaders. Hardware
>> drivers would have to do it in hardware somehow or fallback to Draw.
>> BTW all Radeons can do point size clamping, so adding
>> pipe_context::set_point_size_range(float min, float max) would be okay
>> for them.
>>
>> BTW I think the only operation which can be done before transform
>> feedback is vertex color clamping (as per the EXT_transform_feedback
>> spec).
>>
>> Marek
>
> I probably didn't think of transform feedback when doing this, so this
> looked like a good idea. IIRC most drivers didn't deal correctly (or at
> all) with point size limits - e.g. point size of 0 would disappear
> (instead of showing point size 1 with non-aa points), max/min point size
> ignored etc. and it looked like not all drivers actually could deal with
> it correctly (without adjusting the vertex shader themselves).
> But you're right this can't work correctly with transform feedback (as
> clamping should happen at rasterization time), so feel free to implement
> a solution which works.
> You can't fallback to Draw however (or rather you shouldn't) since you'd
> always need to fallback if points are drawn, because you don't know if
> the actual point size is larger than either the API or implementation
> dependent limit. So you'd need to fix the drivers to handle it themselves.

The only requirement so far seems to be:
If non-aa points, then minimum point size = 1.

I can fix Radeons and Draw. I guess Christoph wants to go with
out-of-spec behavior as usual or do what NVIDIA do. That leaves us
with nvfx and svga. It would be good to know if nvfx can force the
minimum point size to 1 for non-aa points (cc'ing Lucas Stach). Do you
have any idea what we should do for svga?

Marek