On Fri, Apr 24, 2015 at 11:36:27PM +0300, Rogovin, Kevin wrote: > I want to add one more comment on why to replace with the _mesa_geometry_ > functions, which I had thought was so obvious I neglected to mention it: > > With this series the meaning of gl_framebuffer::Width, Height, and so on have > a different meaning. They give the intersection of the backing stores of the > render targets. In contrast, the _mesa_geometry_* functions give the geometry > to feed a rasterizer/windower. By using _mesa_geometry_* functions the code > communicates clearly it wants the geometry to feed windower/rasterizer and > not the geometry of the intersection of the (potentially empty) set of > backing stores. Moreover, it is better to be consistent as well, as later > someone will likely wonder: "why in Gen7 and higher are those _mesa_geometry > functions used but not before?" That question has no good answer because it > does not make sense to not use those functions when programming the > rasterizer/windower thingies. >
Could we split the patch in two? One part dealing with the necessary needed for gen7 and higher to support no_attachments and then another just making older gens consistent. > -Kevin > > -----Original Message----- > From: Rogovin, Kevin > Sent: Friday, April 24, 2015 7:43 PM > To: Pohjolainen, Topi > Cc: mesa-...@freedesktop.org > Subject: RE: [Mesa-dev] [PATCH 5/7] i965: use > _mesa_geometry_width/height/layers/samples for programming geometry of > framebuffer to GEN > > > > My point specifically was that you are also updating atoms that _are not_ > > re-used. > > And as those changes are not really needed, I wouldn't take the risk > > of changing something in vain. I would introduce them only when you have > > patches to really enable older generations. > > My take is the following: > > 1. Tracking (and guaranteeing) that those function left unchanged as is are > exactly just those for before Gen7 is a pain. Much easier, and more reliable > to hit them all instead. A significant number of functions in i965 are not > emit functions of any atom but emit functions of atoms map to them. Again, > more reliable and -safer- to change them all, then just the bare minimum. > > 2. The change is benign. If _HasAttachments is true, then the function > substitution gives the same value. For Gens not supporting the extension > there is no effect. > > 3. Lastly, as stated: for later it leaves the option to enable it for Gen6 > and below, it is just trivial change, but it needs testing on hardware. > > When I writing this work, I originally had it for all Gens, but changed to > support only Gen7and higher because that is all on which I can test it. > > -Kevin > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
