[virglrenderer-devel] [PATCH] shader: Do not adjust y coordonate if an application render to a FBO
Elie Tournier
tournier.elie at gmail.com
Fri Jul 6 08:51:23 UTC 2018
On Wed, Jun 27, 2018 at 07:03:03PM -0700, Gurchetan Singh wrote:
> On Wed, Jun 27, 2018 at 4:18 PM Elie Tournier <tournier.elie at gmail.com> wrote:
> >
> > I almost forget this patch.
> >
> > On Fri, May 25, 2018 at 05:03:51PM -0700, Gurchetan Singh wrote:
> > > On Fri, May 25, 2018 at 7:41 AM Elie Tournier <tournier.elie at gmail.com>
> > > wrote:
> > >
> > > > On Wed, May 16, 2018 at 10:34:29PM +0100, Elie Tournier wrote:
> > > > > Fixes: 7f615cd "shader: Invert y coordonate if on gles host"
> > > >
> > > > Oups. s/coordonate/coordinate/g
> > > > Ping?
> > > > >
> > > > > Signed-off-by: Elie Tournier <elie.tournier at collabora.com>
> > > > > ---
> > > > > src/vrend_renderer.c | 3 +++
> > > > > src/vrend_shader.c | 10 +++++++---
> > > > > 2 files changed, 10 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> > > > > index 55b96db..70ea0c5 100644
> > > > > --- a/src/vrend_renderer.c
> > > > > +++ b/src/vrend_renderer.c
> > > > > @@ -158,6 +158,7 @@ struct vrend_linked_shader_program {
> > > > >
> > > > > GLuint *ubo_locs[PIPE_SHADER_TYPES];
> > > > > GLuint vs_ws_adjust_loc;
> > > > > + GLuint vs_ws_adjust_loc_gles;
> > > >
> > >
> > > nit: should be fs_ws_adjust_loc_gles, since we only emit in the fragment
> > > shader.
> > True
> > >
> > >
> > > > >
> > > > > GLuint fs_stipple_loc;
> > > > >
> > > > > @@ -975,6 +976,7 @@ static struct vrend_linked_shader_program
> > > > *add_shader_program(struct vrend_conte
> > > > > else
> > > > > sprog->fs_stipple_loc = -1;
> > > > > sprog->vs_ws_adjust_loc = glGetUniformLocation(prog_id,
> > > > "winsys_adjust_y");
> > > > > + sprog->vs_ws_adjust_loc_gles = glGetUniformLocation(prog_id,
> > > > "winsys_adjust_y_gles");
> > > > > for (id = PIPE_SHADER_VERTEX; id <= last_shader; id++) {
> > > > > if (sprog->ss[id]->sel->sinfo.samplers_used_mask) {
> > > > > uint32_t mask = sprog->ss[id]->sel->sinfo.samplers_used_mask;
> > > > > @@ -3028,6 +3030,7 @@ void vrend_draw_vbo(struct vrend_context *ctx,
> > > > > return;
> > > > > }
> > > > > glUniform1f(ctx->sub->prog->vs_ws_adjust_loc,
> > > > ctx->sub->viewport_is_negative ? -1.0 : 1.0);
> > > > > + glUniform1f(ctx->sub->prog->vs_ws_adjust_loc_gles,
> > > > ctx->sub->viewport_is_negative ? 0.0 : 1.0);
> > > >
> > >
> > > It looks like vs_ws_adjust_loc and vs_ws_adjust_loc_gles seem to cancel
> > > each other out on GLES (correct me if I'm wrong). Is it possible to just
> > > modify vs_ws_adjust_loc depending on some relevant variables? i.e,
> > >
> > > bool invert = ctx->sub->viewport_is_negative ? true : false;
> > > invert = invert ^ (use_gles && state->sprite_coord_mode
> > > == PIPE_SPRITE_COORD_LOWER_LEFT)
> > We have one on vertex shader and the other one on the fragment shader
> > so they don't cancel each other.
> > >
> > > >
> > > > > if (ctx->sub->rs_state.clip_plane_enable) {
> > > > > for (i = 0 ; i < 8; i++) {
> > > > > diff --git a/src/vrend_shader.c b/src/vrend_shader.c
> > > > > index 1ce18de..05eadd7 100644
> > > > > --- a/src/vrend_shader.c
> > > > > +++ b/src/vrend_shader.c
> > > > > @@ -53,7 +53,7 @@ struct vrend_shader_io {
> > > > > bool glsl_gl_in;
> > > > > bool override_no_wm;
> > > > > bool is_int;
> > > > > - char glsl_name[64];
> > > > > + char glsl_name[128];
> > > >
> > >
> > > This needs a minor rebase, with the arb_gpu_shader5 having landed.
> > Will do in the future patch.
> > >
> > >
> > > > > };
> > > > >
> > > > > struct vrend_shader_sampler {
> > > > > @@ -416,7 +416,7 @@ iter_declaration(struct tgsi_iterate_context *iter,
> > > > > if (iter->processor.Processor == TGSI_PROCESSOR_FRAGMENT) {
> > > > > if (ctx->key->coord_replace & (1 << ctx->inputs[i].sid)) {
> > > > > if (ctx->cfg->use_gles)
> > > > > - name_prefix = "vec4(gl_PointCoord.x, 1.0 -
> > > > gl_PointCoord.y, 0.0, 1.0)";
> > > > > + name_prefix = "vec4(gl_PointCoord.x, mix(1.0 -
> > > > gl_PointCoord.y, gl_PointCoord.y, winsys_adjust_y_gles), 0.0, 1.0)";
> > If you prefer, we can use only one uniform winsys_adjust_y and clamp it like:
> > name_prefix = "vec4(gl_PointCoord.x, mix(1.0 - gl_PointCoord.y, gl_PointCoord.y,
> > clamp(winsys_adjust_y, 0.0, 1.0), 0.0, 1.0)";
> >
> > But I'm not a big fan of this solution. The expression is more complex and I
> > think we have enough memory to store an other uniform.
>
> I prefer the one uniform approach. Also, we don't have to emit the
> uniform every time in the fragment shader -- only when we have point
> primitives.
I didn't find an easy way to know when we have point primitives.
Did you have something in mind?
>
>
> > > > > else
> > > > > name_prefix = "vec4(gl_PointCoord, 0.0, 1.0)";
> > > > > ctx->inputs[i].glsl_predefined_no_emit = true;
> > > > > @@ -444,7 +444,7 @@ iter_declaration(struct tgsi_iterate_context *iter,
> > > > > }
> > > > >
> > > > > if (ctx->inputs[i].glsl_no_index)
> > > > > - snprintf(ctx->inputs[i].glsl_name, 64, "%s", name_prefix);
> > > > > + snprintf(ctx->inputs[i].glsl_name, 128, "%s", name_prefix);
> > > > > else {
> > > > > if (ctx->inputs[i].name == TGSI_SEMANTIC_FOG)
> > > > > snprintf(ctx->inputs[i].glsl_name, 64, "%s_f%d",
> > > > name_prefix, ctx->inputs[i].sid);
> > > > > @@ -2439,6 +2439,10 @@ static char *emit_ios(struct dump_ctx *ctx, char
> > > > *glsl_hdr)
> > > > > bcolor_emitted[0] = bcolor_emitted[1] = false;
> > > > > }
> > > > > if (ctx->prog_type == TGSI_PROCESSOR_FRAGMENT) {
> > > > > + if (ctx->cfg->use_gles) {
> > > > > + snprintf(buf, 255, "uniform float winsys_adjust_y_gles;\n");
> > > > > + STRCAT_WITH_RET(glsl_hdr, buf);
> > > > > + }
> > > > > if (fs_emit_layout(ctx)) {
> > > > > bool upper_left = !(ctx->fs_coord_origin ^
> > > > ctx->key->invert_fs_origin);
> > > > > char comma = (upper_left && ctx->fs_pixel_center) ? ',' : ' ';
> > > > > --
> > > > > 2.17.0
> > > > >
> > > > _______________________________________________
> > > > virglrenderer-devel mailing list
> > > > virglrenderer-devel at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel
> > > >
More information about the virglrenderer-devel
mailing list