pex-renderer icon indicating copy to clipboard operation
pex-renderer copied to clipboard

Where to keep default values

Open vorg opened this issue 2 years ago • 4 comments

There are few variable here and there that if not set might create unexpected or black renders:

  • default roughness: 1 and metallic: 1 are necessary evil in case you provide metallic / roughness texture (ideally they would be roughness=1 and metallic=1

  • emissiveIntensity has to be set (default to at least 1)

  • clearCoatRoughness should be at least 0.04

cachedUniforms.uClearCoatRoughness = material.clearCoatRoughness || 0.04

In the current 2.0 (pre-ECS) implementation those values are scattered across material and renderer with various value || default checks. That becomes a problem even more with ECS where material could be empty object. I wonder what could be unified approach to handling those.

  • how much should be in renderer.material()
  • how much should be internal in the render-system's entity._material or maybe even some entity._material.uniformCache
  • how much can be offloaded to pex-shaders and ifdef everything if not explicitly set in the entity.material e.g. clearCoatRoughness would require USE_CLEAR_COAT_ROUGHNESS or emissiveIntensity would need USE_EMISSIVE_INTENSITY instead of defaulting to forgotten uEmissiveIntensity=0.

vorg avatar Jun 30 '22 13:06 vorg

Philosophically, it sounds more like something a system (render) should worry about. But matching ALL material properties and if/def flags in pex-shaders should also be an objective and could potentially allow for conditionally building shader strings (if we ever tired of all these if/def preprocessor that aren't supported in WebGPU anyway).

dmnsgn avatar Jun 30 '22 14:06 dmnsgn

Practically, not having a fully formed component before the first system pass is an issue.

dmnsgn avatar Jun 30 '22 15:06 dmnsgn

As mentioned in https://github.com/pex-gl/pex-renderer/issues/309 if we remove

entity.material = renderer.material({ baseColor: [1, 0, 0, 1] })
//in favour of
entity.material = { baseColor: [1, 0, 0, 1]} 

how does one discovers a) material b) other props like emissive

docs? system api comments?

vorg avatar Jul 05 '22 11:07 vorg

Maybe the problem is not the component constructors but the name spacing of them. Maybe needing a reference to renderer to call renderer.material is the problem.

Standalone systems work well (if you know which ones needs ctx reference)

module.exports = (node, graph) => {
  const { systems } = require("pex-renderer");

  const triggerIn = node.triggerIn("in");
  const triggerOut = node.triggerOut("out");

  const renderSystem = systems.render({ ctx: graph.ctx });

  triggerIn.onTrigger = (props) => {
    renderSystem.update(props.entities, props.deltaTime);

    node.comment = props.entities.length;

    triggerOut.trigger(props);
  };
};

Maybe the same could work with components

import { components as cmp, entity as createEntity } from "pex-renderer";

const entity = createEntity({ //assigns unique id based on built-in entity counter   
   //assigns identity modelMatrix and empty worldBounds
   transform: cmp.transform({ position: [0, 0, 1] }),

   //assigns default roughness, metallic and sets all textures to null
   material: cmp.material({ baseColor: [1, 1, 1, 1] }) 
})

It gets a bit more problematic with components that need ctx for some of it's data but at least having null properties creates interface for how should they be called

import { components as cmd, entity as createEntity } from "pex-renderer";

const entity = createEntity({
  //assignes reflectionOctMapAtlas to null
  reflectionProbe: cmp.reflectionProbe() 
})

So we know it's reflectionOctMapAtlas not _ reflectionMap or reflectionMap.

Why still pushing for constructors / factory functions.

With e.g. material component it's the renderer that poly-fills it and uses it but with e.g. transform and transform.worldBounds the poly-filling / if null checks would need to happen in transform, render, lightProbe, shadow mapping and possible other systems.

vorg avatar Jul 08 '22 08:07 vorg