[Mesa-dev] [PATCH 2/2] i965: handle gl_PointCoord for Gen4 and Gen5 platform

[Liu Aleaxander]

On Sun, Feb 19, 2012 at 8:54 AM, Eric Anholt <eric at anholt.net> wrote:
> On Sat, 18 Feb 2012 23:07:32 +0800, Liu Aleaxander <aleaxander at gmail.com> wrote:
>> This patch add the support of gl_PointCoord gl builtin var for platform
>> gen4 and gen5(ILK).
>>
>> We can get the point start coord and the current pixel coord from PS
>> payload, and the only left element needed is the point size. Then handle
>> that in SF thread.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=45975
>> Piglit: glsl-fs-pointcoord and fbo-gl_pointcoord
>>
>> NOTE: This is a candidate for stable release branches.
>
>
>> +   /*
>> +    * Window coordinates in an FBO are inverted, which means coord in Y
>> +    * must be inverted, too.
>> +    */
>> +   if ((ctx->Point.SpriteOrigin == GL_LOWER_LEFT) != render_to_fbo) {
>> +      pntcoord.negate = true;
>> +      emit(BRW_OPCODE_ADD, pntcoord, pntcoord, fs_reg(1.0f));
>> +   }
>
> In the FS, it should be looking at the data in the key instead of
> ctx->whatever, to be sure that statechanges are handled correctly.

Yes, I tried this way first, but I have no idea why the value of
origin_upper_left and render_to_fbo in key are both equal to 0
whatever the origin you set and render to fbo or not. I don't check
this issue deeply but that's why I changed to using the value in ctx.

>
> (Though, given that point sprite coordinate flippinng is handled in the
> SF code, I'd think we would handle it the same way for pointcoord too).

It's somehow easy for me to implement this by doing in FS code
generation. If you prefer way do it in SF stage, Ok, I will try to do
it in that way.

>
>> +   /*
>> +    * gl_PointCoord is a FS instead of VS builtin variable, thus is not
>> +    * included in c->nr_setup_regs. But FS need SF do the interpolation,
>> +    * so that here padding the interpolation for gl_PointCoord in last.
>> +    */
>> +   if (c->key.do_point_coord)
>> +      c->nr_setup_regs++;
>
> So you're writing an extra attribute of setup, but you haven't increased
> the size of the URB entry.  If the URB full except for pointcoord, you'd
> end up writing over the next URB entry and probably hanging the GPU.  If
> you correctly allocate the URB size and that gets reflected in
> urb_read_length, you should be able to read your new attribute.

That's maybe the place I don't understand quite well so far. Say, you
write attributes into URB from SF thread to FS. I did increase the
urb_read_length in wm_state. Is that the allocation you mean? Should
I, say, allocate size for extra attributes just for FS in SF stage? If
so, would you please tell me how? Since if I understand correctly, the
urb_read_length in SF_STATE is counted from the attributes from VF
stage. Thus at least urb_read_length in SF stage is not the right
place for me to touch, right?


Thanks,
Yuanhan Liu