[Mesa-dev] [PATCH 4/5] texstore: Add a generic implementation of GL_ARB_clear_texture

Ilia Mirkin imirkin at alum.mit.edu
Mon Jun 16 09:39:50 PDT 2014


On Mon, Jun 16, 2014 at 8:49 AM, Neil Roberts <neil at linux.intel.com> wrote:
> Ilia Mirkin <imirkin at alum.mit.edu> writes:
>
>> Will it? What about the MS case? I thought that for generic you had to
>> do a shader-based approach.
>
> Ah yes, you're right the commit message is nonsense. I guess I should
> also avoid enabling the extension at all until the next patch.
>
>> While I'm sure we all love the
>>
>> while (height --> 0) { // as height goes to 0
>>
>> construct... it's not immediately obvious (to me) that it's correct
>> when just glancing at it. After a few seconds of thought, it is
>> clearly right, but I think the more common thing is to use a for loop
>> where it's obvious there isn't some silly off-by-one error. I don't
>> think this is really used anywhere else in mesa. Here and below.
>
> Ok, yes, I guess it is clearer to use a separate iterator variable.
> However I don't think it's that uncommon in Mesa. Running ‘git grep
> "while (.*--"’ shoes plenty of examples.

git grep 'while.*--' | wc -l
57

Ironically ~half of the instances are in nouveau. I'll go fix those.

$ git grep 'for' | wc -l
50480

I think 'for' is more popular :) [and yeah, it's not a completely fair
comparison, but... people get used to coding patterns.]

>
> I've updated the wip/clear-texture branch on my github with these two
> changes.

Thanks :)

>
> Thanks for the review.
>
> Regards,
> - Neil


More information about the mesa-dev mailing list