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

Hololens 2 rendering issues with Unity and WebXR - Regression? #27286

Closed
De-Panther opened this issue Jul 16, 2020 · 25 comments
Closed

Hololens 2 rendering issues with Unity and WebXR - Regression? #27286

De-Panther opened this issue Jul 16, 2020 · 25 comments

Comments

@De-Panther
Copy link

@De-Panther De-Panther commented Jul 16, 2020

In the current Firefox Reality in the Microsoft Hololens store, AR session works and rendered as intended.
But the nightly build version ( 13/07 ServoApp_1.1.0.0_x64_arm64 ), has some rendering issues:

  • Rendering order.

  • Smearing on transparent background.

I took screenshots from the emulator, but as I understood from people with the device, it's the same.

This is how it looks on the store version, you can see that the rendering order is right, and the transparent background is black (as the emulator doesn't show scene mesh by default):
image

This is how the same AR session looks in the nightly build. You can see the smearing of the numbers and other objects. You can also see some errors in rendering order, the hand model should be near the user and not behind the floating table:
image

This is how it looks when I tried to use WebGLRenderingContext.clear (COLOR_BUFFER_BIT DEPTH_BUFFER_BIT and with and without STENCIL_BUFFER_BIT) at the frame start. There's no smearing, but the hand is rendered behind the floating table:
image

As far as I know, on AR mode we shouldn't use clear at the frame start, as in Handheld and Video see through devices, it has the camera image. If we use clear at the frame start, we won't see the camera image.

@De-Panther
Copy link
Author

@De-Panther De-Panther commented Jul 16, 2020

@jdm
Copy link
Member

@jdm jdm commented Jul 17, 2020

I can reproduce the AR clearing issues on non-UWP builds in our glwindow backend as well.

@jdm
Copy link
Member

@jdm jdm commented Jul 17, 2020

This appears to be a regression from #26927, because the 6/27 nightly build does not show any of these issues. cc @asajeffrey

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 17, 2020

This looks like our old friend depth/stencil. I'll investigate.

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 17, 2020

@jdm when you reproduced with glwindow, was that with a build after #27232 was merged? I'm failing to reproduce with glwindow. I'll try it on UWP.

@jdm
Copy link
Member

@jdm jdm commented Jul 17, 2020

I did not try with glwindow after #27232.

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 17, 2020

OK, that means it's almost certainly depth/stencil. I'll apply a similar fix to the openxr layer manager that I did for the surfman layer manager.

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 17, 2020

I reproduced the issue on the HL2, let's see if I can fix it.

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 17, 2020

OK, I think there's two things going on here. The easy-to-fix one is to add a depth/stencil texture to each XR layer, which is servo/webxr#185. The harder-to-fix one is that there's GL errors being generated, which is what I think is causing the clear errors.

@De-Panther
Copy link
Author

@De-Panther De-Panther commented Jul 17, 2020

Is it something on the Unity WebGL side? I'm limited with how much I can manipulate it

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 17, 2020

No, it's a problem at our end:

RUST: ERROR - canvas::webgl_thread - Last GL operation failed: TexSubImage2D { target: 3553, level: 0, xoffset: 71, yoffset: 54, size: 35x52, format: Alpha, data_type: UnsignedByte, effective_data_type: 5121, unpacking_alignment: 1, alpha_treatment: None, y_axis_treatment: AsIs, pixel_format: None, data: IpcSharedMemory { os... }

GL commands are not meant to fail like this!

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 17, 2020

The tail end of the WebGL commands:

RUST: WARN - canvas::webgl_thread - processing ResizeContext(WebGLContextId(3), 2880x600, WebGLSender(..))
RUST: WARN - canvas::webgl_thread - processing WebGLCommand(WebGLContextId(3), ClearColor(0.0, 0.0, 0.0, 1.0), WebGLCommandBacktrace)
RUST: WARN - canvas::webgl_thread - processing WebGLCommand(WebGLContextId(3), Scissor(0, 0, 1050, 600), WebGLCommandBacktrace)
RUST: WARN - canvas::webgl_thread - processing WebGLCommand(WebGLContextId(3), BindTexture(3553, Some(WebGLTextureId(30))), WebGLCommandBacktrace)
RUST: WARN - canvas::webgl_thread - processing ResizeContext(WebGLContextId(3), 2880x936, WebGLSender(..))
RUST: WARN - canvas::webgl_thread - processing WebGLCommand(WebGLContextId(3), ClearColor(0.0, 0.0, 0.0, 1.0), WebGLCommandBacktrace)
RUST: WARN - canvas::webgl_thread - processing WebGLCommand(WebGLContextId(3), Scissor(0, 0, 1050, 600), WebGLCommandBacktrace)
RUST: WARN - canvas::webgl_thread - processing WebGLCommand(WebGLContextId(3), BindTexture(3553, Some(WebGLTextureId(30))), WebGLCommandBacktrace)
RUST: WARN - script::dom::xrsession - Rendering blank XR frame
RUST: WARN - canvas::webgl_thread - processing WebXRCommand(EndFrame(WebXRLayerManagerId(1), [], WebGLSender(..)))
RUST: WARN - canvas::webgl_thread - processing WebXRCommand(BeginFrame(WebXRLayerManagerId(1), [(ContextId(3), LayerId(0))], WebGLSender(..)))
RUST: WARN - canvas::webgl_thread - processing WebGLCommand(WebGLContextId(3), BindTexture(3553, Some(WebGLTextureId(31))), WebGLCommandBacktrace)
RUST: WARN - canvas::webgl_thread - processing WebGLCommand(WebGLContextId(3), BindFramebuffer(36160, Explicit(WebGLFramebufferId(1))), WebGLCommandBacktrace)
RUST: WARN - canvas::webgl_thread - processing WebGLCommand(WebGLContextId(3), FramebufferTexture2D(36160, 36064, 3553, Some(WebGLTextureId(31)), 0), WebGLCommandBacktrace)
RUST: WARN - canvas::webgl_thread - processing WebGLCommand(WebGLContextId(3), BindFramebuffer(36160, Explicit(WebGLFramebufferId(1))), WebGLCommandBacktrace)
RUST: WARN - canvas::webgl_thread - processing WebGLCommand(WebGLContextId(3), Clear(17664), WebGLCommandBacktrace)
RUST: WARN - canvas::webgl_thread - processing WebGLCommand(WebGLContextId(3), TexSubImage2D { target: 3553, level: 0, xoffset: 129, yoffset: 1, size: 35x52, format: Alpha, data_type: UnsignedByte, effective_data_type: 5121, unpacking_alignment: 1, alpha_treatment: None, y_axis_treatment: AsIs, pixel_format: None, data: IpcSharedMemory { os... }, WebGLCommandBacktrace)
RUST: ERROR - canvas::webgl_thread - Last GL operation failed: TexSubImage2D { target: 3553, level: 0, xoffset: 129, yoffset: 1, size: 35x52, format: Alpha, data_type: UnsignedByte, effective_data_type: 5121, unpacking_alignment: 1, alpha_treatment: None, y_axis_treatment: AsIs, pixel_format: None, data: IpcSharedMemory { os... }
thread 'WebGL thread' panicked at 'Unexpected WebGL error: 0x502 (1282) [TexSubImage2D { target: 3553, level: 0, xoffset: 129, yoffset: 1, size: 35x52, format: Alpha, data_type: UnsignedByte, effective_data_type: 5121, unpacking_alignment: 1, alpha_treatment: None, y_axis_treatment: AsIs, pixel_format: None, data: IpcSharedMemory { os... }]', components\canvas\webgl_thread.rs:2237:17The program '[3716] ServoApp.exe' has exited with code 0 (0x0).
@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 17, 2020

I think what's going on is that when servo begins a frame, it calls glBindTexture, glBindFramebuffer and glFramebufferTexture2D to update the webxr framebuffer, but this interferes with any current bindings of the framebuffer and texture, which then causes glTexSubImage2D to fail.

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 17, 2020

I even wrote a TODO for myself about this at

// TODO: rebind the current bindings

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 17, 2020

OK, I think #27316 should fix this.

@De-Panther
Copy link
Author

@De-Panther De-Panther commented Jul 17, 2020

You people are AWESOME

bors-servo added a commit that referenced this issue Jul 18, 2020
…shearth

Save / restore state when updating opaque framebuffer bindings

<!-- Please describe your changes on the following line: -->

This saves and restores the WebGL bindings for texture and framebuffer when beginning a webxr frame.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #27286
- [x] These changes do not require tests because we don't reftest hololens (it would be nice if we did!)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Jul 20, 2020
…shearth

Save / restore state when updating opaque framebuffer bindings

<!-- Please describe your changes on the following line: -->

This saves and restores the WebGL bindings for texture and framebuffer when beginning a webxr frame.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #27286
- [x] These changes do not require tests because we don't reftest hololens (it would be nice if we did!)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 20, 2020

@De-Panther this fix should be in the next nightly, let us know if this fixed things!

@De-Panther
Copy link
Author

@De-Panther De-Panther commented Jul 20, 2020

@asajeffrey sure!
I guess it would be on nightly build of 2020-07-21 and up? (not sure about the time zone there)
https://download.servo.org/

@jdm
Copy link
Member

@jdm jdm commented Jul 20, 2020

Correct.

@De-Panther
Copy link
Author

@De-Panther De-Panther commented Jul 21, 2020

The render order issue was fixed, but the smearing is still there
https://de-panther.github.io/unity-webxr-export/Build/
image

When clearing the framebuffer at the start of the frame, there's no smearing, but Handheld and Video see through devices require to not clear the framebuffer, as they show the camera image at the background...

@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 21, 2020

OK, reopening the issue.

@asajeffrey asajeffrey reopened this Jul 21, 2020
@jdm jdm added this to the Firefox Reality v1.2 milestone Jul 22, 2020
@asajeffrey asajeffrey mentioned this issue Jul 22, 2020
4 of 4 tasks complete
@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 22, 2020

So hopefully this is fixed by #27373. It looks fixed with the surfman layer manager, I'm building the HL2 one atm.

bors-servo added a commit that referenced this issue Jul 23, 2020
Updated webxr

<!-- Please describe your changes on the following line: -->

Updates webxr to clear the render target every frame.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #27286 (really this time)
- [x] These changes do not require tests because we don't reftest webxr

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jul 23, 2020

The PR which hopefully fixes this has now landed, so the change should be in the next nightly build.

@De-Panther: let me know if it's actually fixed this time!

@De-Panther
Copy link
Author

@De-Panther De-Panther commented Jul 24, 2020

It works!
Thanks!

I just tested on the emulator
image

@De-Panther
Copy link
Author

@De-Panther De-Panther commented Jul 25, 2020

Can also confirm that it works on a device. Got a recording.

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.

3 participants
You can’t perform that action at this time.