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

1.3.0 Hellow WebVR screenshot looks washed out #5014

Closed
diarmidmackenzie opened this issue Feb 24, 2022 · 9 comments
Closed

1.3.0 Hellow WebVR screenshot looks washed out #5014

diarmidmackenzie opened this issue Feb 24, 2022 · 9 comments

Comments

@diarmidmackenzie
Copy link
Contributor

diarmidmackenzie commented Feb 24, 2022

Description:

This is what a screenshot of the Hello Web VR app (Ctrl-Alt-S) used to look like
screenshot--1645741701810

This is what it looks like in 1.3.0 - to my eye this looks far too washed out.
screenshot--1645741354272

This glitch uses a small JS script to run a version of 1.3.0 with the fix for PR4822 reverted, and gives the same screenshot output as 1.2.0 did.

I understand that PR4822 was added because screenshots were coming out too dark in some circumstances, but it seems that the way it's been fixed has just traded one problem for another.

I don't have any expertise around the various WebGLRenderTarget settings, so no idea what the "correct" fix is - but it looks like we still have issues here in getting the screenshot colors right in all cicumstances.

@diarmidmackenzie
Copy link
Contributor Author

Wild guess, from reading 4915 and 4917, is it a question of whether colorManagement is enabled?

The Hello WebVR example doesn't use colorManagement, whereas maybe the cases being looked at in 4915 and 4917 did?

@diarmidmackenzie
Copy link
Contributor Author

Here's a glitch with a totally unmodified "Hello WebVR" except for the addition of the "screenshot" attribute on the scene.
https://fire-vine-family.glitch.me/

Ctrl-Alt-S on this will reproduce the washed out screenshot.

@kfarr
Copy link
Contributor

kfarr commented Feb 25, 2022

Thanks for raising @diarmidmackenzie . I just took a look at the commit (it's been a while). I agree with your analysis.

  • I think the user expectation is "the way my scene looks in the browser is the way the screenshot should look"
  • A proposed solution would be for the screenshot code to respect the scene's colorManagement attribute to properly handle either case. If true then use the current method, if false then omit the line that sets sRGB.

@dmarcos
Copy link
Member

dmarcos commented Feb 25, 2022

Yeah we should match color management on screen and screenshot component. Thanks

@zackdove
Copy link

Is there an easy fix for this in the meantime?

@innovaciones
Copy link

innovaciones commented Aug 18, 2022

Is there an easy fix for this in the meantime?

I tried the code from @diarmidmackenzie and that fixed the problem for me:

<script>      
  AFRAME.components.screenshot.Component.prototype.getRenderTarget = function (width, height) {
      return new THREE.WebGLRenderTarget(width, height, {
        minFilter: THREE.LinearFilter,
        magFilter: THREE.LinearFilter,
        wrapS: THREE.ClampToEdgeWrapping,
        wrapT: THREE.ClampToEdgeWrapping,
        format: THREE.RGBAFormat,
        type: THREE.UnsignedByteType
      });
    }
</script>

I was using the chromakey component and screenshots looked be very washed out, now screenshots looks fine.

@dmarcos
Copy link
Member

dmarcos commented Nov 19, 2022

Any PRs to fix this super welcome

@diarmidmackenzie
Copy link
Contributor Author

I'll have a go.

@dmarcos
Copy link
Member

dmarcos commented Nov 21, 2022

fixed by #5157

@dmarcos dmarcos closed this as completed Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants