[Mesa-dev] [v3 PATCH 06/10] i965: Use _mesa_geometry_ functions appropriately
Matt Turner
mattst88 at gmail.com
Thu May 21 16:14:43 PDT 2015
On Thu, May 21, 2015 at 2:30 PM, <kevin.rogovin at intel.com> wrote:
> From: Kevin Rogovin <kevin.rogovin at intel.com>
>
> Change references to gl_framebuffer::Width, Height, MaxNumLayers
> and Visual::samples to use the _mesa_geometry_ convenience functions
> for those places where the geometry of the gl_framebuffer is needed
> (in contrast to the geometry of the intersection of the attachments
> of the gl_framebuffer).
>
> This patch is to pave the way to enable GL_ARB_framebuffer_no_attachments
> on Gen7 and higher in i965.
>
> v1 -> v2
> Remove changes that would only be active in Gen4/5.
> Type and casting changes for consistency and readability.
>
> v2 -> v3
> Updates for rebase against master
>
> Signed-off-by: Kevin Rogovin <kevin.rogovin at intel.com>
> ---
> src/mesa/drivers/dri/i965/brw_clip_state.c | 9 ++++++---
This is gen4/5 only, isn't it?
> src/mesa/drivers/dri/i965/brw_misc_state.c | 10 +++++++---
> src/mesa/drivers/dri/i965/brw_sf_state.c | 8 ++++++++
As is this?
> src/mesa/drivers/dri/i965/brw_state_upload.c | 6 ++++--
> src/mesa/drivers/dri/i965/brw_wm.c | 7 ++++---
> src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 12 +++++++-----
> src/mesa/drivers/dri/i965/gen6_clip_state.c | 10 +++++++---
> src/mesa/drivers/dri/i965/gen6_multisample_state.c | 3 ++-
> src/mesa/drivers/dri/i965/gen6_scissor_state.c | 13 ++++++++++---
> src/mesa/drivers/dri/i965/gen6_sf_state.c | 3 ++-
> src/mesa/drivers/dri/i965/gen6_viewport_state.c | 5 +++--
> src/mesa/drivers/dri/i965/gen6_wm_state.c | 3 ++-
> src/mesa/drivers/dri/i965/gen7_sf_state.c | 3 ++-
> src/mesa/drivers/dri/i965/gen7_viewport_state.c | 5 +++--
> src/mesa/drivers/dri/i965/gen7_wm_state.c | 3 ++-
> src/mesa/drivers/dri/i965/gen8_viewport_state.c | 8 +++++---
> 16 files changed, 74 insertions(+), 34 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_state.c b/src/mesa/drivers/dri/i965/brw_clip_state.c
> index 3223834..dee74db 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_state.c
> @@ -32,6 +32,7 @@
> #include "brw_context.h"
> #include "brw_state.h"
> #include "brw_defines.h"
> +#include "main/framebuffer.h"
>
> static void
> upload_clip_vp(struct brw_context *brw)
> @@ -59,7 +60,9 @@ brw_upload_clip_unit(struct brw_context *brw)
> struct brw_clip_unit_state *clip;
>
> /* _NEW_BUFFERS */
> - struct gl_framebuffer *fb = ctx->DrawBuffer;
> + const struct gl_framebuffer *fb = ctx->DrawBuffer;
> + const float fb_width = (float)_mesa_geometric_width(fb);
> + const float fb_height = (float)_mesa_geometric_height(fb);
>
> upload_clip_vp(brw);
>
> @@ -127,8 +130,8 @@ brw_upload_clip_unit(struct brw_context *brw)
> /* enable guardband clipping if we can */
> if (ctx->ViewportArray[0].X == 0 &&
> ctx->ViewportArray[0].Y == 0 &&
> - ctx->ViewportArray[0].Width == (float) fb->Width &&
> - ctx->ViewportArray[0].Height == (float) fb->Height)
> + ctx->ViewportArray[0].Width == fb_width &&
> + ctx->ViewportArray[0].Height == fb_height)
> {
> clip->clip5.guard_band_enable = 1;
> clip->clip6.clipper_viewport_state_ptr =
> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c
> index 67a693b..1672786 100644
> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> @@ -39,6 +39,7 @@
> #include "brw_state.h"
> #include "brw_defines.h"
>
> +#include "main/framebuffer.h"
> #include "main/fbobject.h"
> #include "main/glformats.h"
>
> @@ -46,12 +47,15 @@
> static void upload_drawing_rect(struct brw_context *brw)
> {
> struct gl_context *ctx = &brw->ctx;
> + const struct gl_framebuffer *fb = ctx->DrawBuffer;
> + const unsigned int fb_width = _mesa_geometric_width(fb);
> + const unsigned int fb_height = _mesa_geometric_height(fb);
>
> 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.
> OUT_BATCH(0);
> ADVANCE_BATCH();
> }
> @@ -767,7 +771,7 @@ static void upload_polygon_stipple_offset(struct brw_context *brw)
> * works just fine, and there's no window system to worry about.
> */
> if (_mesa_is_winsys_fbo(ctx->DrawBuffer))
> - OUT_BATCH((32 - (ctx->DrawBuffer->Height & 31)) & 31);
> + OUT_BATCH((32 - (_mesa_geometric_height(ctx->DrawBuffer) & 31)) & 31);
> else
> OUT_BATCH(0);
> ADVANCE_BATCH();
> diff --git a/src/mesa/drivers/dri/i965/brw_sf_state.c b/src/mesa/drivers/dri/i965/brw_sf_state.c
> index 014b434..6f9397f 100644
> --- a/src/mesa/drivers/dri/i965/brw_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_sf_state.c
> @@ -52,6 +52,14 @@ static void upload_sf_vp(struct brw_context *brw)
> sizeof(*sfv), 32, &brw->sf.vp_offset);
> memset(sfv, 0, sizeof(*sfv));
>
> + /* 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...
> + */
> + assert(ctx->DrawBuffer->_HasAttachments);
> +
> if (render_to_fbo) {
> y_scale = 1.0;
> y_bias = 0;
> diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c
> index 84b0861..369e7b3 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> @@ -41,6 +41,7 @@
> #include "brw_gs.h"
> #include "brw_wm.h"
> #include "brw_cs.h"
> +#include "main/framebuffer.h"
>
> static const struct brw_tracked_state *gen4_atoms[] =
> {
> @@ -660,6 +661,7 @@ brw_upload_pipeline_state(struct brw_context *brw,
> int i;
> static int dirty_count = 0;
> struct brw_state_flags state = brw->state.pipelines[pipeline];
> + int fb_samples = (int)_mesa_geometric_samples(ctx->DrawBuffer);
The cast looks strange
>
> brw_select_pipeline(brw, pipeline);
>
> @@ -696,8 +698,8 @@ brw_upload_pipeline_state(struct brw_context *brw,
> brw->ctx.NewDriverState |= BRW_NEW_META_IN_PROGRESS;
> }
>
> - if (brw->num_samples != ctx->DrawBuffer->Visual.samples) {
> - brw->num_samples = ctx->DrawBuffer->Visual.samples;
> + if (brw->num_samples != fb_samples) {
> + brw->num_samples = fb_samples;
> brw->ctx.NewDriverState |= BRW_NEW_NUM_SAMPLES;
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
> index 45a03bb..592a729 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -36,6 +36,7 @@
> #include "main/formats.h"
> #include "main/fbobject.h"
> #include "main/samplerobj.h"
> +#include "main/framebuffer.h"
> #include "program/prog_parameter.h"
> #include "program/program.h"
> #include "intel_mipmap_tree.h"
> @@ -462,7 +463,7 @@ static void brw_wm_populate_key( struct brw_context *brw,
> GLuint lookup = 0;
> GLuint line_aa;
> bool program_uses_dfdy = fp->program.UsesDFdy;
> - bool multisample_fbo = ctx->DrawBuffer->Visual.samples > 1;
> + const bool multisample_fbo = _mesa_geometric_samples(ctx->DrawBuffer) > 1;
>
> memset(key, 0, sizeof(*key));
>
> @@ -561,7 +562,7 @@ static void brw_wm_populate_key( struct brw_context *brw,
> * drawable height in order to invert the Y axis.
> */
> if (fp->program.Base.InputsRead & VARYING_BIT_POS) {
> - key->drawable_height = ctx->DrawBuffer->Height;
> + key->drawable_height = _mesa_geometric_height(ctx->DrawBuffer);
> }
>
> if ((fp->program.Base.InputsRead & VARYING_BIT_POS) || program_uses_dfdy) {
> @@ -580,7 +581,7 @@ static void brw_wm_populate_key( struct brw_context *brw,
> key->persample_shading =
> _mesa_get_min_invocations_per_fragment(ctx, &fp->program, true) > 1;
> if (key->persample_shading)
> - key->persample_2x = ctx->DrawBuffer->Visual.samples == 2;
> + key->persample_2x = _mesa_geometric_samples(ctx->DrawBuffer) == 2;
>
> key->compute_pos_offset =
> _mesa_get_min_invocations_per_fragment(ctx, &fp->program, false) > 1 &&
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 160dd2f..1cdc18f 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -35,6 +35,7 @@
> #include "main/mtypes.h"
> #include "main/samplerobj.h"
> #include "program/prog_parameter.h"
> +#include "main/framebuffer.h"
>
> #include "intel_mipmap_tree.h"
> #include "intel_batchbuffer.h"
> @@ -738,6 +739,9 @@ brw_update_renderbuffer_surfaces(struct brw_context *brw,
> uint32_t *surf_offset)
> {
> GLuint i;
> + const int w = (int)_mesa_geometric_width(fb);
> + const int h = (int)_mesa_geometric_height(fb);
> + const int s = (int)_mesa_geometric_samples(fb);
>
> /* Update surfaces for drawing buffers */
> if (fb->_NumColorDrawBuffers >= 1) {
> @@ -748,17 +752,15 @@ brw_update_renderbuffer_surfaces(struct brw_context *brw,
> surf_offset[surf_index] =
> brw->vtbl.update_renderbuffer_surface(
> brw, fb->_ColorDrawBuffers[i],
> - fb->MaxNumLayers > 0, i, surf_index);
> + _mesa_geometric_layers(fb) > 0, i, surf_index);
> } else {
> - brw->vtbl.emit_null_surface_state(
> - brw, fb->Width, fb->Height, fb->Visual.samples,
> + brw->vtbl.emit_null_surface_state(brw, w, h, s,
> &surf_offset[surf_index]);
> }
> }
> } else {
> const uint32_t surf_index = render_target_start;
> - brw->vtbl.emit_null_surface_state(
> - brw, fb->Width, fb->Height, fb->Visual.samples,
> + brw->vtbl.emit_null_surface_state(brw, w, h, s,
> &surf_offset[surf_index]);
> }
> }
> diff --git a/src/mesa/drivers/dri/i965/gen6_clip_state.c b/src/mesa/drivers/dri/i965/gen6_clip_state.c
> index aaf90df..9a29366 100644
> --- a/src/mesa/drivers/dri/i965/gen6_clip_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_clip_state.c
> @@ -31,6 +31,7 @@
> #include "brw_util.h"
> #include "intel_batchbuffer.h"
> #include "main/fbobject.h"
> +#include "main/framebuffer.h"
>
> static void
> upload_clip_state(struct brw_context *brw)
> @@ -145,11 +146,14 @@ upload_clip_state(struct brw_context *brw)
> * the viewport, so we can ignore this restriction.
> */
> if (brw->gen < 8) {
> + const float fb_width = (float)_mesa_geometric_width(fb);
> + const float fb_height = (float)_mesa_geometric_height(fb);
> +
> for (unsigned i = 0; i < ctx->Const.MaxViewports; i++) {
> if (ctx->ViewportArray[i].X != 0 ||
> ctx->ViewportArray[i].Y != 0 ||
> - ctx->ViewportArray[i].Width != (float) fb->Width ||
> - ctx->ViewportArray[i].Height != (float) fb->Height) {
> + ctx->ViewportArray[i].Width != fb_width ||
> + ctx->ViewportArray[i].Height != fb_height) {
> dw2 &= ~GEN6_CLIP_GB_TEST;
> break;
> }
> @@ -179,7 +183,7 @@ upload_clip_state(struct brw_context *brw)
> dw2);
> OUT_BATCH(U_FIXED(0.125, 3) << GEN6_CLIP_MIN_POINT_WIDTH_SHIFT |
> U_FIXED(255.875, 3) << GEN6_CLIP_MAX_POINT_WIDTH_SHIFT |
> - (fb->MaxNumLayers > 0 ? 0 : GEN6_CLIP_FORCE_ZERO_RTAINDEX) |
> + (_mesa_geometric_layers(fb) > 0 ? 0 : GEN6_CLIP_FORCE_ZERO_RTAINDEX) |
> ((ctx->Const.MaxViewports - 1) & GEN6_CLIP_MAX_VP_INDEX_MASK));
> ADVANCE_BATCH();
> }
> diff --git a/src/mesa/drivers/dri/i965/gen6_multisample_state.c b/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> index ec46479..36734f5 100644
> --- a/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_multisample_state.c
> @@ -26,6 +26,7 @@
> #include "brw_context.h"
> #include "brw_defines.h"
> #include "brw_multisample_state.h"
> +#include "main/framebuffer.h"
>
> void
> gen6_get_sample_position(struct gl_context *ctx,
> @@ -34,7 +35,7 @@ gen6_get_sample_position(struct gl_context *ctx,
> {
> uint8_t bits;
>
> - switch (fb->Visual.samples) {
> + switch (_mesa_geometric_samples(fb)) {
> case 1:
> result[0] = result[1] = 0.5f;
> return;
> diff --git a/src/mesa/drivers/dri/i965/gen6_scissor_state.c b/src/mesa/drivers/dri/i965/gen6_scissor_state.c
> index 0111f15..dcbbf70 100644
> --- a/src/mesa/drivers/dri/i965/gen6_scissor_state.c
> +++ b/src/mesa/drivers/dri/i965/gen6_scissor_state.c
> @@ -39,11 +39,14 @@ gen6_upload_scissor_state(struct brw_context *brw)
> const bool render_to_fbo = _mesa_is_user_fbo(ctx->DrawBuffer);
> struct gen6_scissor_rect *scissor;
> uint32_t scissor_state_offset;
> + const int fb_width= (int)_mesa_geometric_width(ctx->DrawBuffer);
> + const int fb_height = (int)_mesa_geometric_height(ctx->DrawBuffer);
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.
>
> scissor = brw_state_batch(brw, AUB_TRACE_SCISSOR_STATE,
> sizeof(*scissor) * ctx->Const.MaxViewports, 32,
> &scissor_state_offset);
>
> +
Extra new line.
> /* _NEW_SCISSOR | _NEW_BUFFERS | _NEW_VIEWPORT */
More information about the mesa-dev
mailing list