Renderers refactor #1850

MichalDybizbanskiCreoox · 2025-04-04T13:02:25Z

No description provided.

xeolabs

Looks good to me - refactoring to use common factories for DRY makes sense.

xeolabs · 2025-06-02T09:52:47Z

examples/scenegraph/materials_MetallicMaterial.html

@@ -70,6 +74,42 @@ <h3>Components used</h3>
        canvasId: "myCanvas"
    });

+    // new ReflectionMap(viewer.scene, {


Let's remove this commented out bit

xeolabs · 2025-06-02T10:33:35Z

src/plugins/XKTLoaderPlugin/XKTLoaderPlugin.js

@@ -1031,6 +1031,21 @@ class XKTLoaderPlugin extends Plugin {
            }
        };

+window._timestamp = (function() {


Looks like window._timeStamp is no longer needed

Right, it's one of the things left temporarily for the draft, as it's easier to debug with.
It'll be gone in the final PR.

xeolabs · 2025-06-02T12:24:27Z

src/viewer/scene/mesh/emphasis/EmphasisShaderSource.js

+                 ? lightSetup.directionalLights.map(
+                     light => `(max(dot(${attributes.normal.view}, ${light.getDirection(geometry.viewMatrix, attributes.position.view)}), 0.0) * ${light.getColor()})`)
+                 : [ ]);
+        // TODO: A blending mode for emphasis materials, to select add/multiply/mix


I agree - dynamic blending mode for emphasis materials

paireks · 2025-06-03T13:22:09Z

examples/scenemodel/vbo_instancing_geometries.html

@@ -285,11 +286,20 @@ <h3>Components Used</h3>

    const map = {};

+    const RNG = function(seed) {


This function is introduced in this PR in many examples to replace Math.random(), but I don't think it makes sense here, as examples should be simple. But if it's really needed for some reason, then it would be good to at least refer to the source. I'm not sure where the original source comes from, if this is the one: https://stackoverflow.com/a/72732727, then we should refer to it, as it is under CC BY-SA 4.0.

This function has been introduced temporarily for reproducibility during debugging.
There's a few commits there in the branch (especially at its beginning) that were meant only for development, and will be gone in the final PR.

paireks

Just a comment, that it is great case to use automatic tests Ania and Mateusz prepared for us. Not only to make sure it works after refactor, but also to check their work and see if these tests makes sense. I'd ask Mateusz once he's back to try to run them before merge.

MichalDybizbanskiCreoox · 2025-06-06T10:57:31Z

@xeolabs I've pushed the latest commits added on top of the branch that we discussed last time.
Besides internal refactorings they migrate SAO renderers and Occlusion Tester to the createProgramVariablesState abstraction.
96c7c94...7e980ed

MichalDybizbanskiCreoox · 2025-06-23T10:56:32Z

The following relates to the current HEAD at 5d128a5.

This PR refactors renderers architecture, but in general doesn't change rendered outputs.
There is one exception to that, namely the
9d0fa08 "FIX TrianglesColorTextureRenderer's uColorMap sRGBToLinear decoding (to make the same as e.g. TrianglesPBRRenderer)"
commit changes how texels are processed in the shaders, which results in changes in the outputs.
The tests affected by this change are all 8 from the tests/snapshots-loading-textures.spec.js set.

By the end of the PR three commits are added that reintroduce those previously reverted changes.
At the last commit of the PR (5d128a5) all tests render the same outputs as the ones rendered with the master branch, except for the 8 snapshots-loading-textures tests mentioned above.

…Source

…s failing randomness

…reverted at the beginning of this branch Note that the "cleaner edges" had originally only been implemented for VBO's and DTX's Color and FlatColor programs. The support probably needs to be extended to some of the other programs, as well as to the Mesh hierarchy.

…es layers)" #1884 - reverted at the beginning of this branch

…ntextrestored handlers

…ext lose handling

…} into renderSnapshotToCanvas

…hotWithPlugins

…nderer

MichalDybizbanskiCreoox requested a review from xeolabs April 4, 2025 13:02

MichalDybizbanskiCreoox force-pushed the renderers-refactor branch 2 times, most recently from 1d32a74 to e41f600 Compare May 17, 2025 18:27

MichalDybizbanskiCreoox force-pushed the renderers-refactor branch from e41f600 to 998b102 Compare May 27, 2025 11:41

jakubcreoox force-pushed the master branch from f3c14eb to ce67c09 Compare May 28, 2025 14:19

xeolabs approved these changes Jun 2, 2025

View reviewed changes

paireks reviewed Jun 3, 2025

View reviewed changes

MichalDybizbanskiCreoox force-pushed the renderers-refactor branch from 7e980ed to 5d128a5 Compare June 23, 2025 10:51

MichalDybizbanskiCreoox force-pushed the renderers-refactor branch from 5d128a5 to e7d6c8c Compare June 28, 2025 10:45

MichalDybizbanskiCreoox force-pushed the renderers-refactor branch 3 times, most recently from 2cad949 to e1c0f92 Compare July 8, 2025 09:56

MichalDybizbanskiCreoox force-pushed the renderers-refactor branch 4 times, most recently from f5c04fe to 5caed07 Compare August 4, 2025 16:31

MichalDybizbanskiCreoox added 11 commits August 12, 2025 18:35

Move getHash from renderers to shaderSources

84d8be3

Replace mesh Renderers with instantiateMeshRenderer

af61460

Inline MeshRenderer

43e8585

Move material type hashing into wrapRenderer

aa2d4fc

CHANGE all renderers to rely on mesh._geometry._state.hash

0209775

Remove unused mesh argument from {PickMesh,PickTriangle,Shadow}Shader…

1a2e2b8

…Source

Remove unused appendFragmentOutputs "vWorldPosition" argument

33b2f91

Replace createGammaOutputSetup by useGammaOutput

4854032

Split setupInputs into setupMeshInputs and setupPickClipPosInputs

925ebe2

Abstract DrawShaderSource lights as lightSetup object

50eafa3

Migrate DrawShaderSource lightSetup to use lazyShaderUniform

f434a12

MichalDybizbanskiCreoox added 28 commits August 12, 2025 18:35

Simplify snap picking by not using FrameCtx::snapPickLayerNumber

58da76e

Replace snapPickResult.sort by snapPickResult.reduce

15c55c9

Simplify snap picking

6fd92b0

Simplify Renderer

40e0601

Move internalformats from RenderBuffer::bind to constructor

da65f02

Move canvas-independent framebuffers to local variables

e73e136

Get rid of unnecessary RenderBuffer's canvas property

2b440e8

Use resetPickFrameCtx from Renderer::pick and ::snapPick

8594ebf

Remove unused FrameContext::this.pickElements{Count,Offset}

ce529c7

Remove unused by snapPick FrameContext::viewParams::{far,near}

56fa125

FIX the Renderer::pick up vector calculation, to not rely on sometime…

203ef48

…s failing randomness

Reapply PR "Allow to read geometry directly from the GPU (VBO triangl…

56377ae

…es layers)" #1884 - reverted at the beginning of this branch

Remove unused {bind,unbind}OutputFrameBuffer variables

8091770

Remove unused Renderer::render opaqueOnly parameter

482261c

REMOVE Canvas::fire("webglcontextrestored") event and partial webglco…

90db545

…ntextrestored handlers

REMOVE Canvas::fire("webglcontextlost") event and adjust Scene's cont…

5faa20d

…ext lose handling

Inline readArray inside Renderer

cfb678c

Move snapshot-related Renderer methods inside the snapshot property

00331af

Only localize Renderer::snapshot definition

951a14b

Move readImage and readImageAsCanvas inside Renderer from RenderBuffer

e53732e

Combine Renderer::{renderSnapshot, readSnapshot, readSnapshotAsCanvas…

1d6b8bb

…} into renderSnapshotToCanvas

Abstract Viewer::_renderSnapshotToCanvas for getSnapshot and getSnaps…

e6f062d

…hotWithPlugins

Simplify Renderer::snapshot::{beginSnapshot,endSnapshot}

f74fe62

Simplify Renderer::render interface

c273aac

Make SAODepthLimitedBlurRenderer and SAOOcclusionRenderer similar

05d11c8

Inline SAODepthLimitedBlurRenderer and SAOOcclusionRenderer inside Re…

8e7b12a

…nderer

Set package.json version 2.6.88-renderers-refactor-v5

af6f353

MichalDybizbanskiCreoox force-pushed the renderers-refactor branch from 5b5e8a9 to af6f353 Compare August 12, 2025 16:37

chore: Release v2.6.88-renderers-refactor-v5 dist files

45f3c54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Renderers refactor #1850

Renderers refactor #1850

Uh oh!

MichalDybizbanskiCreoox commented Apr 4, 2025

Uh oh!

xeolabs left a comment

Uh oh!

xeolabs Jun 2, 2025

Uh oh!

xeolabs Jun 2, 2025

Uh oh!

MichalDybizbanskiCreoox Jun 4, 2025

Uh oh!

xeolabs Jun 2, 2025

Uh oh!

paireks Jun 3, 2025

Uh oh!

MichalDybizbanskiCreoox Jun 4, 2025

Uh oh!

paireks left a comment

Uh oh!

MichalDybizbanskiCreoox commented Jun 6, 2025

Uh oh!

MichalDybizbanskiCreoox commented Jun 23, 2025 •

edited

Loading

Uh oh!

Uh oh!

		@@ -285,11 +286,20 @@ <h3>Components Used</h3>

		const map = {};

		const RNG = function(seed) {

Renderers refactor #1850

Are you sure you want to change the base?

Renderers refactor #1850

Uh oh!

Conversation

MichalDybizbanskiCreoox commented Apr 4, 2025

Uh oh!

xeolabs left a comment

Choose a reason for hiding this comment

Uh oh!

xeolabs Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

xeolabs Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

MichalDybizbanskiCreoox Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

xeolabs Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

paireks Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

MichalDybizbanskiCreoox Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

paireks left a comment

Choose a reason for hiding this comment

Uh oh!

MichalDybizbanskiCreoox commented Jun 6, 2025

Uh oh!

MichalDybizbanskiCreoox commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

MichalDybizbanskiCreoox commented Jun 23, 2025 •

edited

Loading