>> src/mesa/drivers/dri/i965/brw_misc_state.c | 10 +++++++--- > > src/mesa/drivers/dri/i965/brw_sf_state.c | 8 ++++++++
> As is this? brw_misc_state.c is active for all Gens. The change to brw_sf_state.c is to add a comment warning that using gl_framebuffer::Width and so on is only ok if _HasAttachments is false. > BEGIN_BATCH(4); > OUT_BATCH(_3DSTATE_DRAWING_RECTANGLE << 16 | (4 - 2)); > OUT_BATCH(0); /* xmin, ymin */ > - OUT_BATCH(((ctx->DrawBuffer->Width - 1) & 0xffff) | > - ((ctx->DrawBuffer->Height - 1) << 16)); > + OUT_BATCH(((fb_width - 1) & 0xffff) | > + ((fb_height - 1) << 16)); > Remove the tab on this line and indent it properly while we're changing it. Actually it fits on one line, so that will be the change :) > + /* Accessing the fields Width and Height of > + * gl_framebuffer to produce the values to > + * program the viewport and scissor is fine > + * as long as the gl_framebuffer has atleast > + * one attachment. > Line wrapping... OK. > struct brw_state_flags state = brw->state.pipelines[pipeline]; > + int fb_samples = (int)_mesa_geometric_samples(ctx->DrawBuffer); > The cast looks strange Is there a spacing missing in the cast? or is it strange because of the types? > These casts look weird. (Happens elsewhere in this patch too). > Assuming brw_context::num_samples doesn't need to be signed, I'd be > inclined to change its type to unsigned and remove the casts. Grepping > for 'num_samples = ' shows some other instances of us implicitly > converting unsigned <-> int. I was hesitant to be changing types in the patch series. I wanted to minimize the changes and try to keep it consistent with what is left unchanged. If the types should be unsigned int, I can do that, but I would like to hear the opinions of others too. > Extra new line. OK. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
