mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
10.87k stars 2.19k forks source link

Shader minification #9390

Open karimnaaji opened 4 years ago

karimnaaji commented 4 years ago

Changes to the shaders seems to be rather impactful on the library size. To minimize that we can try to make use of https://www.npmjs.com/package/glsl-minifier which also benefits from a forked version of https://github.com/aras-p/glsl-optimizer.

glsl-optimizer has a few advantages such as dead code removal, algebraic simplifications (increase of instruction merging), constant propagation, constant folding and inlining or loop unfolding.

If we manage to introduce both of these we would most likely benefit from both library size reduction and potential run-time GPU optimization.

We would have to find a way to flag our shader injection symbols to not be optimized, as we have shader code variants based on data driven styling.

cc @arindam1993

mourner commented 4 years ago

Note that we used glsl-optimizer at the beginning but dropped it 5 years ago due to constant portability problems and instability: https://github.com/cutting-room-floor/glsl-optimizer/issues/5#issuecomment-83828425

It might have improved since then though, worth a try. I have added rudimentary minification in https://github.com/mapbox/mapbox-gl-js/pull/7368 that strips comments/whitespace, and I vaguely recall trying out more aggressive minification and getting minuscule improvements over this basic one, then choosing the simpler option. We should definitely use it if we can get a substantial reduction though.

karimnaaji commented 4 years ago

@mourner that's good to info. I was flagged by that after modifying a few shaders, where the diff was not significant but the increase quite large. Do you know if there is something similar to https://github.com/google/bloaty for JS to visualize chunk sizes and get a better idea of where the bits are going?

mourner commented 4 years ago

@karimnaaji https://github.com/danvk/source-map-explorer is similar and I've used it a little, although I'm not sure if it's accurate enough (I've seen some weird results with it) because it depends on good source mapping.

arindam1993 commented 4 years ago

https://www.npmjs.com/package/rollup-plugin-visualizer seemed pretty good to me. It integrates easily as a rollup plugin.

karimnaaji commented 4 years ago

I gave this a try to see where this could get us.

Prior integration of glsl-optimizer used node gyp, which can easily break over time if the bindings aren't maintained. To fix that I instead built a version with emscripten and directly invoked the function at build time https://github.com/mapbox/mapbox-gl-js/commit/faa3758945b6081826b1ebdea5cdc89b389cf8fe#diff-07d7789c86b2e39e5a38052b72e13551R71. This has the advantage of only depending on one c function end point.

Other issues that I came around is that our shaders aren't compilable right away because of data-driven styling and injections. This can be fixed by injecting both prelude functions signatures and dds variable in the shader symbol table: https://github.com/karimnaaji/glsl-optimizer/commit/f21e2551a29e1e2f305182f930c479c64ed29513.

Good news is all render tests are passing with this prebuilt emscripten version: https://app.circleci.com/pipelines/github/mapbox/mapbox-gl-js/5216/workflows/e46666bf-d60d-48ed-96a0-89afb7bef120/jobs/73026

Concerning the size, it's not a win. It could be pushed further as we introduce more shader and disable loop unrolling (which duplicates a lot of data when used).

On runtime, I haven't profiled on mobile to see if there are any improvements in GPU timings, but it might be worthwhile to see.

Some issues/concerns involved if we decide to pursue this:

JannikGM commented 3 years ago

In a fork of mapbox, I'm using a modified shader which contains procedural textures for symbols.

I currently have trouble minifying our input shaders, because of the #pragma mapbox stuff (as already mentioned by @karimnaaji). So I'd be interested in having a better solution within mapbox which is aware of these things.

I'm not only interested in minification, but also optimization (as there is a considerable amount of dead-code and non-optimized code in my shaders).

From https://github.com/mapbox/mapbox-gl-js/issues/9390#issuecomment-596576505 (@mourner):

I have added rudimentary minification in #7368 that strips comments/whitespace, and I vaguely recall trying out more aggressive minification

These minification steps are already too aggressive, because they break my shaders. I currently have to turn off some minification steps:

// barebones GLSL minification
if (minify) {
    code = code.trim() // strip whitespace at the start/end
        .replace(/\s*\/\/[^\n]*\n/g, '\n') // strip double-slash comments
        .replace(/\n+/g, '\n') // collapse multi line breaks
        .replace(/\n\s+/g, '\n') // strip identation
        //.replace(/\s?([+-\/*=,])\s?/g, '$1') // strip whitespace around operators (THIS BREAKS MY SHADERS!)
        //.replace(/([;\(\),\{\}])\n(?=[^#])/g, '$1'); // strip more line breaks (THIS BREAKS MY SHADERS!)
}

As my shader is also generated from some DCC tool it's not easy to debug. However, I currently assume that some macro breaks. I'm currently using #define (no multi-line, but function macros), #if, #endif. At this time, I can't share my shaders, and I'm not sure when I'll find time to analyze this issue. I'll create a separate github issue if I do.

However, these shortcomings of the shader minifcation should be documented, and more complicated shaders (than present in vanilla mapbox) should potentially be considered when picking an optimizer / minifier.