aframe icon indicating copy to clipboard operation
aframe copied to clipboard

1.3.0 Hellow WebVR screenshot looks washed out

Open diarmidmackenzie opened this issue 2 years ago • 6 comments

Description:

  • A-Frame Version: 1.3.0
  • Platform / Device: Chrome browser
  • Reproducible Code Snippet or URL: https://glitch.com/edit/#!/sly-sixth-topaz?path=index.html%3A19%3A9

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 avatar Feb 24 '22 22:02 diarmidmackenzie

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 avatar Feb 24 '22 22:02 diarmidmackenzie

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.

diarmidmackenzie avatar Feb 24 '22 22:02 diarmidmackenzie

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.

kfarr avatar Feb 25 '22 00:02 kfarr

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

dmarcos avatar Feb 25 '22 22:02 dmarcos

Is there an easy fix for this in the meantime?

zackdove avatar Apr 14 '22 20:04 zackdove

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.

innovaciones avatar Aug 18 '22 21:08 innovaciones

Any PRs to fix this super welcome

dmarcos avatar Nov 19 '22 03:11 dmarcos

I'll have a go.

diarmidmackenzie avatar Nov 19 '22 11:11 diarmidmackenzie

fixed by https://github.com/aframevr/aframe/pull/5157

dmarcos avatar Nov 21 '22 01:11 dmarcos