pex-gl / pex-renderer

Physically based renderer (PBR) and scene graph for PEX.
https://pex-gl.github.io/pex-renderer/examples/index.html
MIT License
237 stars 16 forks source link

Camera.backgroundColor no longer working #211

Closed vorg closed 1 year ago

vorg commented 5 years ago

It’s because it has been moved to post processing along with all pases and shaders. That while valid from code point of view makes less sense from API side.

vorg commented 5 years ago

Background color in Postprocessing#L56

This backgroundColor is used in _drawFrameFboCommand which is then used in Renderer.draw

   if (postProcessingCmp && postProcessingCmp.enabled) {
        ctx.submit(postProcessingCmp._drawFrameFboCommand, () => {
          this.drawMeshes(camera, false, null, null, skyboxes[0], false)
        })
      } else {
        ctx.submit({ viewport: camera.viewport }, () => {
          this.drawMeshes(camera, false, null, null, skyboxes[0], true)
        })
      }

That means we currently can't set bg color without postprocessing

const cameraEntity = renderer.add(
  renderer.entity([
    renderer.postProcessing({
      // enabled: false,
      ssao: true,
      fxaa: false,
      dof: true,
      dofDepth: 18,
      backgroundColor: [0.2, 0.2, 0.2, 1.0] //wrong place
    }),
    renderer.camera({
      fov: Math.PI / 3,
      aspect: ctx.gl.drawingBufferWidth / ctx.gl.drawingBufferHeight,
      exposure: 2,
      backgroundColor: [0.2, 0.2, 0.2, 1.0] //would be better? or is camera just a lens?
    }),
    renderer.orbiter({
      position: [0, 2, 20]
    })
  ])
)

Another way to approach it is rename backgroundColor and rename to clearColor which would match the pass, we can still put it in Camera but also default to null / transparent. Although that's also incorrect because clearing with transparent color is a valid webgl operation.. so I do think we should keep default to black [0, 0, 0, 1]

vorg commented 5 years ago

So turns out we don't clear background at all if no postprocessing is enabled. Examples like lights work only because by default WebGL context preserveDrawingBuffer is set to false.

vorg commented 5 years ago

Looking at WWTJD (what would three.js do) they have Scene.background which makes things more clear.

vorg commented 5 years ago

The problem with background color and PBR is that it would have to take exposure, tonemapping and linear space into account. Moving it to Skybox.backgroundColor would allow scenes that correctly reflect that light. Screenshot 2019-07-16 at 12 36 48

This can be achieved already with single color texture. assigned to Skybox.texture

vorg commented 5 years ago

To sum up the options

  1. Keep it in Postprocessing (no, because without postprocessing we don't clear bg color at all now)
  2. Move it to Camera (feels wrong, camera is just pure optics now)
  3. Move it to Skybox (most accurate from PBR point of view but bloats Skybox component)
  4. Renderer.clearColor (simplest position, not PBR accurate but does the job, i see conflict with postprocessing that needs to have it's FBOs cleared with something... would it borrow color from renderer?)
simonharrisco commented 5 years ago

Seems a lot to have to add a skybox just to change background colour....

vorg commented 5 years ago

@simonharrisco if you want it to automatically make it gamma corrected, over exposed and tone-mapped then yes... otherwise

renderer.clearColor = someColor
   .map((c) => Math.pow(c, 2.2) * cameraCmp.exposure)
   .map((c => c / (1 + c)))
vorg commented 5 years ago

Still if i had to vote I would probably go for Renderer.clearColor because simplicity... the only thing is we don't have runtime modifiable renderer props... everything is a setting passed to constructor and stays like that in internal _state object

vorg commented 5 years ago

Proposal

renderer.set({
  clearColor: [1, 0, 0, 1]
})

This pattern renderer.set could be useful for setting shadow quality and other things in the future.

dmnsgn commented 1 year ago

Implemented here 72b8e1d1ea6b7f647422c1d6e47b5e3bd73adf24 With fix 8f5660be1567ee48667a294245827de3244fdfb4

dmnsgn commented 1 year ago

v4: added clearColor