Skip to content

[GL] Store GLEmulation.materialShininess as a scalar number. NFC - #28107

Merged
sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:glemu-material-shininess
Oct 30, 2026
Merged

sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:glemu-material-shininess

Conversation

@sbc100

@sbc100 sbc100 commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator

Unlike the other material properties (materialAmbient, materialDiffuse, materialSpecular, and materialEmission), which are 4-component vectors passed to GLctx.uniform4fv, materialShininess is a single float passed to GLctx.uniform1f.

Store GLEmulation.materialShininess directly as a number initialized to 0 (matching other scalar uniforms such as fogStart and alphaTestRef) instead of wrapping it in a 1-element Float32Array. This also avoids a Closure Compiler type warning on GLEmulation.materialShininess[0] when compiling with -sEXPORT_ES6.

Split out from #27747

@sbc100
sbc100 requested a review from kripken October 29, 2026 22:49

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the materialShininess property in src/lib/libglemu.js from a single-element Float32Array to a primitive number initialized to 0. While this simplifies the code, a review comment correctly points out that completely removing the initialization line during state reset can lead to state leakage across context resets. It is recommended to explicitly reset GLEmulation.materialShininess to 0 in the re-initialization block.

Comment thread src/lib/libglemu.js
Unlike the other material properties (`materialAmbient`,
`materialDiffuse`, `materialSpecular`, and `materialEmission`), which
are 4-component vectors passed to `GLctx.uniform4fv`,
`materialShininess` is a single float passed to `GLctx.uniform1f`.

Store `GLEmulation.materialShininess` directly as a number initialized
to `0` (matching other scalar uniforms such as `fogStart` and
`alphaTestRef`) instead of wrapping it in a 1-element `Float32Array`.
This also avoids a Closure Compiler type warning on
`GLEmulation.materialShininess[0]` when compiling with `-sEXPORT_ES6`.

Split out from emscripten-core#27747
@sbc100
sbc100 force-pushed the glemu-material-shininess branch from 4ea223e to 43fb8c1 Compare October 29, 2026 22:58
@sbc100
sbc100 enabled auto-merge (squash) October 29, 2026 23:04
@sbc100
sbc100 disabled auto-merge October 30, 2026 16:14
@sbc100
sbc100 merged commit cb54724 into emscripten-core:main Oct 30, 2026
42 checks passed
@sbc100
sbc100 deleted the glemu-material-shininess branch October 30, 2026 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants