ShaderCodeCursor. Optimization of lines parsing #13935

Dok11 · 2023-06-02T18:48:46Z

I notices that set lines takes 19.29ms for 68 calls while my scene initializaion, so I add some checks to avoid process heavy operations like trim and split on the string. As a result, in my case this function take 9.45ms for same 68 calls. And it still works as before, of course, but faster :)

P.S. Results of my measurements:

default:

{count: 68, avg: 0.342647, max: 1.500000, allTime: 23.299999}
{count: 68, avg: 0.267647, max: 1.300000, allTime: 18.199999}
{count: 68, avg: 0.282352, max: 1.299999, allTime: 19.200000}
{count: 68, avg: 0.294117, max: 1.200000, allTime: 19.999999}
{count: 68, avg: 0.330882, max: 1.700000, allTime: 22.500000}
{count: 68, avg: 0.326470, max: 4.500000, allTime: 22.199999}
{count: 68, avg: 0.276470, max: 1.100000, allTime: 18.799998}
{count: 68, avg: 0.255882, max: 0.799999, allTime: 17.400000}
{count: 68, avg: 0.263235, max: 1.200000, allTime: 17.900000}
{count: 68, avg: 0.260294, max: 0.899999, allTime: 17.699999}
{count: 68, avg: 0.269117, max: 1.000000, allTime: 18.299999}
{count: 68, avg: 0.280882, max: 1.099999, allTime: 19.099999}
{count: 68, avg: 0.260294, max: 0.900000, allTime: 17.700000}
{count: 68, avg: 0.263235, max: 0.900000, allTime: 17.899999}

avg allTime: 19.29ms

-------------------------------------------------------------

after optimization:

{count: 68, avg: 0.195588, max: 3.000000, allTime: 13.299999}
{count: 68, avg: 0.138235, max: 1.000000, allTime:  9.399999}
{count: 68, avg: 0.180882, max: 2.000000, allTime: 12.300000}
{count: 68, avg: 0.155882, max: 1.400000, allTime: 10.600000}
{count: 68, avg: 0.151470, max: 0.900000, allTime: 10.299999}
{count: 68, avg: 0.145588, max: 1.599999, allTime:  9.899999}
{count: 68, avg: 0.120588, max: 0.699999, allTime:  8.199999}
{count: 68, avg: 0.122058, max: 0.700000, allTime:  8.299999}
{count: 68, avg: 0.117647, max: 0.700000, allTime:  8.000000}
{count: 68, avg: 0.123529, max: 0.799999, allTime:  8.400000}
{count: 68, avg: 0.135294, max: 0.800000, allTime:  9.200000}
{count: 68, avg: 0.141176, max: 0.700000, allTime:  9.600000}
{count: 68, avg: 0.138235, max: 1.000000, allTime:  9.400000}
{count: 68, avg: 0.133823, max: 0.799999, allTime:  9.099999}
{count: 68, avg: 0.117647, max: 0.600000, allTime:  8.000000}
{count: 68, avg: 0.150000, max: 1.899999, allTime: 10.200000}
{count: 68, avg: 0.114705, max: 0.900000, allTime:  7.799999}
{count: 68, avg: 0.135294, max: 0.700000, allTime:  9.199999}
{count: 68, avg: 0.123529, max: 0.700000, allTime:  8.399999}
{count: 68, avg: 0.138235, max: 0.699999, allTime:  9.400000}

avg allTime: 9.45ms

bjsplat · 2023-06-02T18:50:12Z

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

bjsplat · 2023-06-02T19:13:34Z

Snapshot stored with reference name:
refs/pull/13935/merge

Test environment:
https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13935/merge/index.html

To test a playground add it to the URL, for example:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13935/merge/index.html#WGZLGJ#4600

Links to test babylon tools with this snapshot:

https://playground.babylonjs.com/?snapshot=refs/pull/13935/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/13935/merge
https://gui.babylonjs.com/?snapshot=refs/pull/13935/merge
https://nme.babylonjs.com/?snapshot=refs/pull/13935/merge

To test the snapshot in the playground with a playground ID add it after the snapshot query string:

https://playground.babylonjs.com/?snapshot=refs/pull/13935/merge#BCU1XR#0

bjsplat · 2023-06-02T19:45:08Z

Visualization tests for webgl2 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13935/merge/testResults/webgl2/index.html

If tests were successful afterwards, this report might not be available anymore.

bjsplat · 2023-06-04T15:06:02Z

Visualization tests for webgl2 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13935/merge/testResults/webgl2/index.html

If tests were successful afterwards, this report might not be available anymore.

RaananW · 2023-06-05T09:06:58Z

There is a shader compilation issue in the webgl2 visualization tests

bjsplat · 2023-06-05T20:18:06Z

Visualization tests for webgl2 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13935/merge/testResults/webgl2/index.html

If tests were successful afterwards, this report might not be available anymore.

Dok11 · 2023-06-08T20:10:47Z

There is a shader compilation issue in the webgl2 visualization tests

@RaananW as I can see in logs, there is an error with npm install:
https://dev.azure.com/babylonjs/ContinousIntegration/_build/results?buildId=25611&view=logs&j=1f876048-d530-54e9-d49b-d5fedfaadf19&t=6aa1df57-0aab-5db6-f94c-8054e58bee0c

/usr/bin/bash --noprofile --norc /home/vsts/work/_temp/73c5cb84-7ea8-4efc-8191-59f6177859ff.sh
npm ERR! code 1
npm ERR! path /home/vsts/work/1/s/node_modules/puppeteer
npm ERR! command failed
npm ERR! command sh -c -- node install.js
npm ERR! Requesting latest Firefox Nightly version from https://product-details.mozilla.org/1.0/firefox_versions.json
npm ERR! ERROR: Failed to set up Firefox Nightly r116.0a1! Set "PUPPETEER_SKIP_DOWNLOAD" env variable to skip download.
npm ERR! Error: Download failed: server returned code 404. URL: https://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/firefox-116.0a1.en-US.linux-x86_64.tar.bz2
npm ERR!     at /home/vsts/work/1/s/node_modules/puppeteer/lib/cjs/puppeteer/node/BrowserFetcher.js:371:27
npm ERR!     at ClientRequest.requestCallback (/home/vsts/work/1/s/node_modules/puppeteer/lib/cjs/puppeteer/node/BrowserFetcher.js:500:13)
npm ERR!     at Object.onceWrapper (node:events:628:26)
npm ERR!     at ClientRequest.emit (node:events:513:28)
npm ERR!     at HTTPParser.parserOnIncomingClient (node:_http_client:693:27)
npm ERR!     at HTTPParser.parserOnHeadersComplete (node:_http_common:128:17)
npm ERR!     at TLSSocket.socketOnData (node:_http_client:534:22)
npm ERR!     at TLSSocket.emit (node:events:513:28)
npm ERR!     at addChunk (node:internal/streams/readable:315:12)
npm ERR!     at readableAddChunk (node:internal/streams/readable:289:9)

Is this related to my updates?

RaananW · 2023-06-08T20:24:49Z

There is a shader compilation issue in the webgl2 visualization tests

@RaananW as I can see in logs, there is an error with npm install:
https://dev.azure.com/babylonjs/ContinousIntegration/_build/results?buildId=25611&view=logs&j=1f876048-d530-54e9-d49b-d5fedfaadf19&t=6aa1df57-0aab-5db6-f94c-8054e58bee0c

/usr/bin/bash --noprofile --norc /home/vsts/work/_temp/73c5cb84-7ea8-4efc-8191-59f6177859ff.sh
npm ERR! code 1
npm ERR! path /home/vsts/work/1/s/node_modules/puppeteer
npm ERR! command failed
npm ERR! command sh -c -- node install.js
npm ERR! Requesting latest Firefox Nightly version from https://product-details.mozilla.org/1.0/firefox_versions.json
npm ERR! ERROR: Failed to set up Firefox Nightly r116.0a1! Set "PUPPETEER_SKIP_DOWNLOAD" env variable to skip download.
npm ERR! Error: Download failed: server returned code 404. URL: https://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/firefox-116.0a1.en-US.linux-x86_64.tar.bz2
npm ERR!     at /home/vsts/work/1/s/node_modules/puppeteer/lib/cjs/puppeteer/node/BrowserFetcher.js:371:27
npm ERR!     at ClientRequest.requestCallback (/home/vsts/work/1/s/node_modules/puppeteer/lib/cjs/puppeteer/node/BrowserFetcher.js:500:13)
npm ERR!     at Object.onceWrapper (node:events:628:26)
npm ERR!     at ClientRequest.emit (node:events:513:28)
npm ERR!     at HTTPParser.parserOnIncomingClient (node:_http_client:693:27)
npm ERR!     at HTTPParser.parserOnHeadersComplete (node:_http_common:128:17)
npm ERR!     at TLSSocket.socketOnData (node:_http_client:534:22)
npm ERR!     at TLSSocket.emit (node:events:513:28)
npm ERR!     at addChunk (node:internal/streams/readable:315:12)
npm ERR!     at readableAddChunk (node:internal/streams/readable:289:9)

Is this related to my updates?

Feels like a temporary npm issue. I have triggered it again, let's see if it fails with the same problem again

bjsplat · 2023-06-08T20:57:06Z

Visualization tests for webgl2 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13935/merge/testResults/webgl2/index.html

If tests were successful afterwards, this report might not be available anymore.

bjsplat · 2023-06-09T17:49:17Z

Visualization tests for webgl2 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13935/merge/testResults/webgl2/index.html

If tests were successful afterwards, this report might not be available anymore.

Dok11 · 2023-06-11T21:14:23Z

@RaananW could you help me to find what is this test?

Searcing "Glossiness" by sources does not give image similar to

Most similar is

from PBRSpecularGlossinessMaterial.png

I trying to find the playground with bugs where I can check this PR changes.

bjsplat · 2023-06-11T22:41:56Z

Visualization tests for webgl2 have failed. If some tests failed because the snapshots do not match, the report can be found at

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13935/merge/testResults/webgl2/index.html

If tests were successful afterwards, this report might not be available anymore.

RaananW · 2023-06-12T08:47:10Z

@RaananW could you help me to find what is this test?

Searcing "Glossiness" by sources does not give image similar to

Most similar is

from PBRSpecularGlossinessMaterial.png

I trying to find the playground with bugs where I can check this PR changes.

i'll repeat my previous statement :-)

There is a shader compilation issue.

    ERR BJS - [22:24:41]: #version 300 es
    #define WEBGL2 
    #define NUM_BONE_INFLUENCERS 0
    #define NUM_MORPH_INFLUENCERS 0
    #define SHADER_NAME vertex:[object Object]
    precision highp float;
            attribute vec3 position;
            out vec3 Position;
    void main(void) {
                gl_Position = vec4(position, 1.);
                Position = position * 0.5 + 0.5;
    }

      at log (packages/tools/tests/test/visualization/visualization.utils.ts:50:21)
          at runMicrotasks (<anonymous>)

  console.log
    ERR BJS - [22:24:41]: Fragment code:

      at log (packages/tools/tests/test/visualization/visualization.utils.ts:50:21)
          at runMicrotasks (<anonymous>)

  console.log
    ERR BJS - [22:24:41]: #version 300 es
    #define WEBGL2 
    #define NUM_BONE_INFLUENCERS 0
    #define NUM_MORPH_INFLUENCERS 0
    #define SHADER_NAME fragment:[object Object]
    precision highp float;
            precision highp samplerCube;
            precision highp sampler2DArray;
            in vec3 Position;
            uniform sampler2DArray Texture2DArray;
            uniform samplerCube TextureCubeMap;
            uniform sampler2D Texture2D;
    layout(location = 0) out vec4 glFragColor;
    void main(void) {
    if (Position.x < 0.5 && Position.y < 0.5) {
                    glFragColor = textureLod(Texture2DArray, vec3(Position.xy, 2), 0.);
    } else if (Position.x < 0.5 && Position.y >= 0.5) {
                    glFragColor = textureLod(Texture2DArray, vec3(Position.xy, 4), 0.);
    } else if (Position.x >= 0.5 && Position.y < 0.5) {
                    glFragColor = textureLod(TextureCubeMap, vec3(Position.x, -2.0, Position.y), 0.);
    } else {
                    glFragColor = textureLod(Texture2D, vec2(Position.xy), 0.);
    }
    }

      at log (packages/tools/tests/test/visualization/visualization.utils.ts:50:21)
          at runMicrotasks (<anonymous>)

  console.log
    ERR BJS - [22:24:41]: Offending line [7] in vertex code:         attribute vec3 position;

So the tests either time out (because they can't render) or fail (for the same reason). Is it running locally correctly with webgl2?

Also - please be sure to merge from master and run npm install. I have made some changes to the vis-tests process. It won't fix the issue. but will nonetheless use the right dependencies.

Popov72 · 2023-06-15T13:53:45Z

The test is (you can find it by looking for the test name - Glossiness Debug Mode - in packages/tools/tests/test/visualization/config.json):

https://playground.babylonjs.com/#2FDQT5#2281

The expected output is the one on the right in the visualization test report, so it is:

RaananW · 2023-06-26T11:53:59Z

Any update here?

Dok11 · 2023-06-26T12:16:21Z

Sorry, I'll back here when I have time, probably, it will be after several weeks.
I still have interest to finish these changes =)

Popov72 · 2023-07-11T13:41:08Z

Line 49, this._lines.push(line); should be this._lines.push(trimmedLine); to fix the compilation problem reported by @RaananW in https://playground.babylonjs.com/#XSNYAU#22.

However, there's a problem with the else if (semicolonIndex === line.length - 1 || semicolonIndex === trimmedLine.length - 1) { test.

It's possible that there's a ";" embedded in a line and that the second part of the condition will return true (which is not what we want, in the case of multiple ";" in a line we should fallback to the split and the loop).

For eg:

    const vertex = `
                       attribute vec3 position; varying vec3 Position;

        void main(void) {
            gl_Position = vec4(position, 1.);
            Position = position * 0.5 + 0.5;
        }
    `;

If line = " attribute vec3 position; varying vec3 Position;", semicolonIndex = line.indexOf(";") = 46 and semicolonIndex is different from line.length - 1 (which is 69). But the second part of the condition is true, because the trimmed line is "attribute vec3 position; varying vec3 Position;" and trimmedLine.length - 1 == 46!

Here's the PG corresponding to this case: https://playground.babylonjs.com/#XSNYAU#28

However, I wonder why you added this second part in the condition, it seems to me that we don't need it?

Dok11 · 2023-07-11T14:36:08Z

However, I wonder why you added this second part in the condition, it seems to me that we don't need it?

I don't remember it for now, but it seems like it was different changes in different time. Now looks like we should change the else if (semicolonIndex === line.length - 1 || semicolonIndex === trimmedLine.length - 1) { to else if (semicolonIndex === trimmedLine.length - 1) { because here we have trimmed line anyway. Probably, sometime before here was inline .trim() call and it was reasonable to hide additional calculation (trim call) to second part of || check, but not now.

Line 49, this._lines.push(line); should be this._lines.push(trimmedLine); to fix the compilation problem reported by

Thanks! I'll check it later today and push changes also into PR.

Popov72 · 2023-07-11T15:15:37Z

else if (semicolonIndex === trimmedLine.length - 1) {

is ok if you change const semicolonIndex = line.indexOf(";"); to const semicolonIndex = trimmedLine.indexOf(";");

sebavan · 2023-07-11T22:05:29Z

Cannot wait, thanks @Dok11 !!!

sebavan · 2023-07-11T22:05:56Z

and @Popov72 of course ;-)

BabylonJS#13935 (comment)

ShaderCodeCursor. Optimization of lines parsing

459eea9

Popov72 approved these changes Jun 4, 2023

View reviewed changes

BabylonJS deleted a comment from RaananW Jun 5, 2023

ShaderCodeCursor. Fix the branch of "Semicolon at the end of the line"

d702d7c

BabylonJS#13935 (comment)

sebavan merged commit f7f9b95 into BabylonJS:master Jul 12, 2023
9 checks passed

Popov72 mentioned this pull request Jul 14, 2023

Shaders: Fix shader parsing #14049

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ShaderCodeCursor. Optimization of lines parsing #13935

ShaderCodeCursor. Optimization of lines parsing #13935

Dok11 commented Jun 2, 2023 •

edited

Loading

bjsplat commented Jun 2, 2023

bjsplat commented Jun 2, 2023

bjsplat commented Jun 2, 2023

bjsplat commented Jun 4, 2023

RaananW commented Jun 5, 2023

bjsplat commented Jun 5, 2023

Dok11 commented Jun 8, 2023

RaananW commented Jun 8, 2023

bjsplat commented Jun 8, 2023

bjsplat commented Jun 9, 2023

Dok11 commented Jun 11, 2023

bjsplat commented Jun 11, 2023

RaananW commented Jun 12, 2023

Popov72 commented Jun 15, 2023

RaananW commented Jun 26, 2023

Dok11 commented Jun 26, 2023

Popov72 commented Jul 11, 2023

Dok11 commented Jul 11, 2023

Popov72 commented Jul 11, 2023

sebavan commented Jul 11, 2023

sebavan commented Jul 11, 2023

ShaderCodeCursor. Optimization of lines parsing #13935

ShaderCodeCursor. Optimization of lines parsing #13935

Conversation

Dok11 commented Jun 2, 2023 • edited Loading

bjsplat commented Jun 2, 2023

bjsplat commented Jun 2, 2023

bjsplat commented Jun 2, 2023

bjsplat commented Jun 4, 2023

RaananW commented Jun 5, 2023

bjsplat commented Jun 5, 2023

Dok11 commented Jun 8, 2023

RaananW commented Jun 8, 2023

bjsplat commented Jun 8, 2023

bjsplat commented Jun 9, 2023

Dok11 commented Jun 11, 2023

bjsplat commented Jun 11, 2023

RaananW commented Jun 12, 2023

Popov72 commented Jun 15, 2023

RaananW commented Jun 26, 2023

Dok11 commented Jun 26, 2023

Popov72 commented Jul 11, 2023

Dok11 commented Jul 11, 2023

Popov72 commented Jul 11, 2023

sebavan commented Jul 11, 2023

sebavan commented Jul 11, 2023

Dok11 commented Jun 2, 2023 •

edited

Loading