yuq / mesa-lima

Deprecated, new place: https://gitlab.freedesktop.org/lima
https://github.com/yuq/mesa-lima/wiki
165 stars 17 forks source link

lima: Implement scissor support #10

Closed PabloPL closed 6 years ago

PabloPL commented 6 years ago

For scissor 0,0,0,0 we're skipping any drawings, for other cases it's calculated the same way as limadriver. Had to add support for tgsi shaders - it's done in the same way as vc4/freedreno (we can convert them from tgsi to nir) and use nir_opt_global_to_local and nir_lower_regs_to_ssa nir pass.

PabloPL commented 6 years ago

Still wasn't able to run gbm-surface-color, when i tried to enable scissors by adding

glEnable(GL_SCISSOR_TEST);
glScissor(0, 0, 0, 0); // or anything other than (0, 0, TARGET_SIZE, TARGET_SIZE)

In this case mesa is generating tgsi shader, so previously it was failing at assert(cso->type == PIPE_SHADER_IR_NIR);

Now it's complaining

gbm-surface: nir/nir_opt_undef.c:87: opt_undef_vecN: Assertion `alu->dest.dest.is_ssa' failed

Any ideas how to solve this? I don't have any mesa experience but wanted to gather some by looking at task "command stream setup for other OGL purpose like clear, blend, depth, and etc." from wiki. Looking at lima code, scissors should be easiest from them

yuq commented 6 years ago

You're welcome to work on this topic. As for this patch, it looks good to me. But I'll wait it be more complete before merge it, that means either this patch or a patch serial that finally achieve your goal to enable the scissor support.

This problem should be the original lima_program_optimize_vs_nir lima_program_optimize_fs_nir are not written for TGSI converted NIR, so may miss some pass for it, you can reference the vc4 vc4_shader_state_create for the additional pass like NIR_PASS_V(s, nir_opt_global_to_local); NIR_PASS_V(s, nir_lower_regs_to_ssa);

yuq commented 6 years ago

BTW. if we have to use NIR from TGSI, maybe we need to implement PIPE_CAP_TGSI_VS_LOWER_VIEWPORT_TRANSFORM in TGSI too, because I just add it for NIR only.

yuq commented 6 years ago

Do the two new commits finish your scissor support?

PabloPL commented 6 years ago

Adding nir_opt_global_to_local and nir_lower_regs_to_ssa pass to both fs and vs solved problem with compiling shader. But i found some problems with last patch and need to make more testing/work on it

PabloPL commented 6 years ago

PR rebased against 17.3.

PabloPL commented 6 years ago

About TGSI path - i wasn't able to reproduce it using gbm-surface-color, but was able to reproduce it using piglit tests (just compile piglit with libgbm installed and then run).

PIGLIT_PLATFORM=gbm ./bin/gl-1.0-scissor-copypixels -auto -fbo

I had to make following change in lima_screen.c

case PIPE_SHADER_CAP_MAX_TEXTURE_SAMPLERS:
      return 0; // return 16; /* need investigate */

(because we don't have yet create_sampler_view implemented and it was causing a crash).

yuq commented 6 years ago

Good, seems we may use piglit to get a TODO list for mesa-lima now. I'll merge your TGSI change for now and mark the TGSI viewport change later implement. But it should not bother your scissor patch test in gbm-surface-color. So please go ahead for the scissor patch fix.

PabloPL commented 6 years ago

What if in the second frame, the scissor is enabled but not changed, so this won't be dirty but still need be set?

By using rasterizer.scissor field to check if scissor test is enabled this is fixed. During testing (comparing with images generated using the same gbm-surface-move on intel gpu and lima) i've found that it's fixed. But there is issue with zero scissor with multiple frames. With following dummy change

+       if (si == 1) {
+               glScissor(0,0, 0, 0);
+               glEnable(GL_SCISSOR_TEST);
+       } else if (si == 4) {
+               glDisable(GL_SCISSOR_TEST);
+       }

        //glDrawElements(GL_TRIANGLES, sizeof(index)/sizeof(GLuint), GL_UNSIGNED_INT, index);
        glDrawArrays(GL_TRIANGLES, 0, 3);

On intel:

Again, need to work more on this...

PabloPL commented 6 years ago

For now i'm closing this PR (till all issues with scissor are fixed).