[Mesa-dev] [PATCH 5/7] i965: use _mesa_geometry_width/height/layers/samples for programming geometry of framebuffer to GEN

[Rogovin, Kevin]

> I read the patch again and I'm still in the opinion that the changes to the 
> pure pre-gen7 logic (i.e., logic that is not re-used for later gens) are not needed.

As I have tried and apparently failed to communicate, it is -better- and more consistent. Need 
is a far stronger word. Without a doubt, if the extension is never enabled for those older
Gens, then it does not matter in terms of produced output. However, I stated that it leaves 
a trap and an inconsistency which I find quite bothering.

> The shared logic between pre-gen7 and later, namely setup for renderbuffers, drawing rectangle and 
> fragment shader compilation key are safe to do as they only introduce new logic that is conditional to 
> no-attachments being used.

And that is exactly for the case for that code that is not shared. Indeed, if the shared code is safe 
for pre-Gen7, then so is the non-shared code. 

> Your concern about the readers getting confused could be also addressed with assert(brw->gen >= 7) 
>  and a comment saying that the no-attachment specific path is not applicable for older gens.

There is only one occurrence of "no-attachment specific code paths" in these i965 patches
and that is associated to scissoring.  The rest is existing code is changed from accessing Width, 
Height of gl_framebuffer to "getting" those values from a function. There is no proper place
to insert an assert(brw->gen >=7 ), since, with the exception of the scissoring (and it is just
one if block) there is no such "no attachment code path". I had thought the diffs of the series
made that quite clear.

> And when it comes to the pure pre-gen7 logic, I, in fact, have just the opposite opinion on making it to go through the no-attachment-aware path.
> As the extension is not possible for older gens, I find it clearer that logic explicitly by-passes such paths that even consider it.

Um, I am pretty sure than pre Gen7 hardware can do the extension. The crux is that the extension
is pointless for such hardware because pre Gen7 hardware does not (AFAIK) have a feature that
allows for a fragment shader to have a side effect. Even that statement is not totally true. Indeed,
one can argue performance queries and occlusion queries with framebuffer_no_attachments 
make some form of sense (it would give an application a count of sorts). 

-Kevin