Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clipping issues on Android: textureSize(sCache, 0) is 0 #907

Closed
MortimerGoro opened this issue Feb 20, 2017 · 4 comments
Closed

Clipping issues on Android: textureSize(sCache, 0) is 0 #907

MortimerGoro opened this issue Feb 20, 2017 · 4 comments
Assignees

Comments

@MortimerGoro
Copy link
Contributor

@MortimerGoro MortimerGoro commented Feb 20, 2017

Hello,

I'm working on WebVR Servo for Android and couldn't get some basic WebGL canvas to show on a simple html. It also happens with other non WebGL simple elements as images. After some debugging found that the textures from images and WebGL canvases are ok but the problem is related to the clipping implementation.

In the ps_image.fs.gl shader, the textureLod(sColor0, st, 0.0) gives the correct color but it's multiplied by a 0 alpha. The alpha 0 is set by the call to do_clip() function.

 **alpha = min(alpha, do_clip());**

    // We calculate the particular tile this fragment belongs to, taking into
    // account the spacing in between tiles. We only paint if our fragment does
    // not fall into that spacing.
    vec2 position_in_tile = mod(relative_pos_in_rect, vStretchSize + vTileSpacing);
    // We clamp the texture coordinates to the half-pixel offset from the borders
    // in order to avoid sampling outside of the texture area.
    vec2 st = vTextureOffset + ((position_in_tile / vStretchSize) * vTextureSize);
    st = clamp(st, vStRect.xy, vStRect.zw);

    alpha = alpha * float(all(bvec2(step(position_in_tile, vStretchSize))));

oFragColor = vec4(1.0, 1.0, 1.0, alpha) * textureLod(sColor0, st, 0.0);

write_clip() function is correctly called in the vertex shader. The problem seems to be that textureSize(sCache, 0).xy returns a vec(0, 0) value, which leads to an invalid alpha value. In the same demo run on my desktop machine, textureSize(sCache, 0).xy returns a vec(1, 1) value, which leads to a correct alpha value.

  • Where is sCache texture bound? I checked some OpenGL calls and it seemed that both Desktop & Android have a 0 texture_id bound to sCache (in the GL_TEXTURE4 unit). Should sCache be bound to an active texture in a simple demo or that only happens with multiple render passes in more complex demos?
  • By the way I have also checked to call textureSize() of a new sampler2DArray uniform which is initialized to 0 and not used anywhere else. The Desktop OpenGL textureSize call still returns a vec(1,1) size vs Android textureSize call a vec(0,0) size.
@kvark
Copy link
Member

@kvark kvark commented Feb 21, 2017

Thanks @MortimerGoro !
So basically we are not setting sCache properly in some cases. Do you mind sharing the test case?

I see that we are setting it in load_program, which seems fine:

        let u_cache = gl::get_uniform_location(program.id, "sCache");
        if u_cache != -1 {
            gl::uniform_1i(u_cache, TextureSampler::Cache as i32);
        }
@kvark
Copy link
Member

@kvark kvark commented Feb 21, 2017

And indeed we are not always actually binding it. We only bind it for the passes following the first one. I suggest we bind a dummy texture for the first pass each time.

@kvark kvark self-assigned this Feb 21, 2017
@MortimerGoro
Copy link
Contributor Author

@MortimerGoro MortimerGoro commented Feb 21, 2017

@kvark the testcase I've using is the test_webgl_triangle.html file on Servo tests/html folder. It also happens with a plain <img src="test.png"> in a simple html page, with no javascript.

I see that we are setting it in load_program, which seems fine:

    let u_cache = gl::get_uniform_location(program.id, "sCache");
    if u_cache != -1 {
        gl::uniform_1i(u_cache, TextureSampler::Cache as i32);
    }

Yes that uniform is correctly set on Android. I think that the texture bind wasn't done on Desktop OpenGL too. I put some log calls and only saw that 0 texture_id was bound. But it seems that textureSize(texure_id = 0) leads to undefined behavior on the shader, gives (1,1) on desktop and (0,0) on Android.

@kvark
Copy link
Member

@kvark kvark commented Feb 21, 2017

Hold tight, fix is coming ;)
FYI, this is one of the cases that a bind-less API like gfx-rs would prevent at compile time.

bors-servo added a commit that referenced this issue Feb 22, 2017
Dummy sCache texture input

Fixes #907

r? @glennw

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/913)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Feb 22, 2017
Dummy sCache texture input

Fixes #907

r? @glennw

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/913)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.