Changes done and pushed.
On Tue, Nov 25, 2014 at 10:17 PM, Kenneth Graunke <kenn...@whitecape.org> wrote: > On Tuesday, November 25, 2014 10:05:18 PM Chris Forbes wrote: >> Fixes broken rendering in Windows-based QtQuick2 apps run through Wine. >> This library sets all texture units' GL_COORD_REPLACE, leaves point >> sprite mode enabled, and then draws a triangle fan. >> >> Will need a slightly different fix for Gen4-5, but I don't have my old >> machines in a usable state currently. >> >> V2: - Simplify patch -- the real changes are no longer duplicated across >> the Gen6 and Gen7 atoms. >> - Also don't clobber attr overrides -- which matters on Haswell too, >> and fixes the other half of the problem >> - Fix newly-introduced warnings >> >> Signed-off-by: Chris Forbes <chr...@ijw.co.nz> >> Cc: "10.4" <mesa-sta...@lists.freedesktop.org> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84651 >> --- >> src/mesa/drivers/dri/i965/gen6_sf_state.c | 54 >> ++++++++++++++++++++++++------- >> src/mesa/drivers/dri/i965/gen7_sf_state.c | 3 +- >> 2 files changed, 45 insertions(+), 12 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/gen6_sf_state.c >> b/src/mesa/drivers/dri/i965/gen6_sf_state.c >> index 24d2754..778c6d3 100644 >> --- a/src/mesa/drivers/dri/i965/gen6_sf_state.c >> +++ b/src/mesa/drivers/dri/i965/gen6_sf_state.c >> @@ -129,6 +129,18 @@ get_attr_override(const struct brw_vue_map *vue_map, >> int urb_entry_read_offset, >> } >> >> >> +static bool >> +is_drawing_points(const struct brw_context *brw) >> +{ >> + /* Determine if the primitives *reaching the SF* are points */ >> + const struct gl_context *ctx = &brw->ctx; > > It doesn't seem like BRW_NEW_PRIMITIVE and BRW_NEW_VUE_MAP_GEOM_OUT > are enough to cover the use of OutputType. I think you want: > > if (brw->geometry_program) { > /* BRW_NEW_GEOMETRY_PROGRAM */ > return brw->geometry_program->OutputType == GL_POINTS; > } else { > /* BRW_NEW_PRIMITIVE */ > return brw->primitive == _3DPRIM_POINTLIST; > } > > and then include BRW_NEW_GEOMETRY_PROGRAM in the atom's dirty flags. > > (Using brw->geometry_program should be equivalent to > ctx->GeometryProgram._Current, but we may as well use the brw copy for > consistency with the rest of the code. It's also more obviously connected > to the BRW_NEW_GEOMETRY_PROGRAM flag.) > >> + if (ctx->GeometryProgram._Current) >> + return ctx->GeometryProgram._Current->OutputType == GL_POINTS; >> + else >> + return brw->primitive == _3DPRIM_POINTLIST; >> +} >> + >> + >> /** >> * Create the mapping from the FS inputs we produce to the previous pipeline >> * stage (GS or VS) outputs they source from. >> @@ -149,6 +161,23 @@ calculate_attr_overrides(const struct brw_context *brw, >> /* _NEW_LIGHT */ >> bool shade_model_flat = brw->ctx.Light.ShadeModel == GL_FLAT; >> >> + /* From the Ivybridge PRM, Vol 2 Part 1, 3DSTATE_SBE, >> + * description of dw10 Point Sprite Texture Coordinate Enable: >> + * >> + * "This field must be programmed to zero when non-point primitives >> + * are rendered." >> + * >> + * The SandyBridge PRM doesn't explicitly say that point sprite enables >> + * must be programmed to zero when rendering non-point primitives, but >> + * the IvyBridge PRM does, and if we don't, we get garbage. >> + * >> + * This is not required on Haswell, as the hardware ignores this state >> + * when drawing non-points -- although we do still need to be careful to >> + * correctly set the attr overrides. >> + */ >> + /* BRW_NEW_PRIMITIVE | _NEW_PROGRAM */ >> + bool drawing_points = is_drawing_points(brw); >> + >> /* Initialize all the attr_overrides to 0. In the loop below we'll >> modify >> * just the ones that correspond to inputs used by the fs. >> */ >> @@ -167,18 +196,20 @@ calculate_attr_overrides(const struct brw_context *brw, >> >> /* _NEW_POINT */ >> bool point_sprite = false; >> - if (brw->ctx.Point.PointSprite && >> - (attr >= VARYING_SLOT_TEX0 && attr <= VARYING_SLOT_TEX7) && >> - brw->ctx.Point.CoordReplace[attr - VARYING_SLOT_TEX0]) { >> - point_sprite = true; >> + if (drawing_points) { >> + if (brw->ctx.Point.PointSprite && >> + (attr >= VARYING_SLOT_TEX0 && attr <= VARYING_SLOT_TEX7) && >> + brw->ctx.Point.CoordReplace[attr - VARYING_SLOT_TEX0]) { >> + point_sprite = true; >> + } >> + >> + if (attr == VARYING_SLOT_PNTC) >> + point_sprite = true; >> + >> + if (point_sprite) >> + *point_sprite_enables |= (1 << input_index); >> } >> >> - if (attr == VARYING_SLOT_PNTC) >> - point_sprite = true; >> - >> - if (point_sprite) >> - *point_sprite_enables |= (1 << input_index); >> - >> /* flat shading */ >> if (interp_qualifier == INTERP_QUALIFIER_FLAT || >> (shade_model_flat && is_gl_Color && >> @@ -411,7 +442,8 @@ const struct brw_tracked_state gen6_sf_state = { >> _NEW_MULTISAMPLE), >> .brw = (BRW_NEW_CONTEXT | >> BRW_NEW_FRAGMENT_PROGRAM | > > Add BRW_NEW_GEOMETRY_PROGRAM here, and please add them in alphabetical order. > >> - BRW_NEW_VUE_MAP_GEOM_OUT), >> + BRW_NEW_VUE_MAP_GEOM_OUT | >> + BRW_NEW_PRIMITIVE), > > > >> .cache = CACHE_NEW_WM_PROG >> }, >> .emit = upload_sf_state, >> diff --git a/src/mesa/drivers/dri/i965/gen7_sf_state.c >> b/src/mesa/drivers/dri/i965/gen7_sf_state.c >> index 109b825..371b9de 100644 >> --- a/src/mesa/drivers/dri/i965/gen7_sf_state.c >> +++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c >> @@ -93,7 +93,8 @@ const struct brw_tracked_state gen7_sbe_state = { >> _NEW_PROGRAM), >> .brw = (BRW_NEW_CONTEXT | >> BRW_NEW_FRAGMENT_PROGRAM | >> - BRW_NEW_VUE_MAP_GEOM_OUT), >> + BRW_NEW_VUE_MAP_GEOM_OUT | >> + BRW_NEW_PRIMITIVE), > > Ditto - BRW_NEW_GEOMETRY_PROGRAM as well, alphabetize. > >> .cache = CACHE_NEW_WM_PROG >> }, >> .emit = upload_sbe_state, >> > > Otherwise, this looks great to me - thanks for tracking down this obscure bug! > > Assuming you make those fixes, > Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
