On 14 December 2011 13:42, Marek Olšák <<a href="mailto:maraeo@gmail.com">maraeo@gmail.com</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I think RASTERIZER_DISCARD has nothing to do with transform feedback. I think it's part of the same spec because it's not useful without it. As I understand it, _NEW_TRANSFORM_FEEDBACK is dirty when transform feedback buffer bindings are changed or just enabled/disabled. On the other hand, RASTERIZER_DISCARD enables or disables the rasterizer, so it should fall into the same category as face culling for example. (I even implemented it using face culling on r600) Also there would be no way to know whether _NEW_TRANSFORM_FEEDBACK changes just TFB buffer bindings, or just RASTERIZER_DISCARD, or both. Marek </blockquote><div> I see where you are coming from--I could have implemented rasterizer discard on i965 gen6 in the same way. However, I think there are three compelling reasons to consider rasterizer discard to be related to transform feedback: (1) from a user perspective, it really only makes sense to use rasterizer discard when transform feedback is active. Thus, it's highly likely that when the rasterizer discard state is changed, transform feedback settings will be changed too. (2) rasterizer discard functionality is described by the same set of extensions that enable transform feedback (e.g. EXT_transform_feedback), so presumably the inventors of these two features thought they were closely related. (3) the enable bit that Mesa uses to keep track of the state of rasterizer discard is in gl_context::TransformFeedback, not gl_context::Transform. Item (3) is the most important to me. One of the scarier things about i965 driver development is that we have to manually keep track of which hardware configuration commands correspond to which dirty bits. If we miss a dependency, that causes a subtle bug in which a state change might not cause the hardware state to be updated properly. These kinds of bugs are not well exposed by Piglit tests, because most Piglit tests make a large number of state changes followed by a draw operation, so a missing dependency might easily go undetected. The thing that allows me to sleep at night is that there is a nice one-to-one correspondence between almost all of the dirty bits and corresponding substructures of gl_context. For example, the _NEW_MODELVIEW dirty bit corresponds to gl_context::ModelView, _NEW_PROJECTION corresponds to gl_context::Projection, and so on. That means any time I am worried that I'm not handling dirty bits correctly, I can visually inspect the code and make sure that the dirty bits I'm consulting match up with which elements of gl_context I'm accessing. If we leave the code as is, then there's a big undocumented exception to that one-to-one correspondence, wherein gl_context::TransformFeedback.RasterDiscard is covered by _NEW_TRANSFORM, not _NEW_TRANSFORM_FEEDBACK, as one would logically guess based on where it's located within gl_context. I'm not confident that I (or other developers) will remember this exception once we're no longer in the thick of implementing transform feedback and rasterizer discard. It seems to me that we have three possible approaches to choose from here: (a) Go ahead with this patch, and modify r600 code to recompute the face culling mode when the _NEW_TRANSFORM_FEEDBACK dirty bit is set. (b) Don't apply this patch, and instead move RasterDiscard from gl_context::TransformFeedback to gl_context::Transform, so that we restore the one-to-one correspondence between dirty bits and substructures of gl_context. (c) Do nothing, and rely on programmers to remember that RasterDiscard is an exception to the usual correspondence between dirty bits and substructures of gl_context. I'm really not comfortable with (c) because of the risk of future bugs. I suppose I could be talked into (b) if there's popular support for it, but it's not my favourite, because as I said earlier, I think there are actually a lot of good reasons to think of rasterizer discard as related to transform feedback. My preference is to do (a). </div></div>