Skip to content

Fix floating-point/vector remainder and operator precedence - #1031

Merged
kblaschke merged 1 commit into
projectM-visualizer:masterfrom
johnneerdael:fix/hlsl-floating-point-remainder
Sep 30, 2026
Merged

kblaschke merged 1 commit into
projectM-visualizer:masterfrom
johnneerdael:fix/hlsl-floating-point-remainder

Conversation

@johnneerdael

@johnneerdael johnneerdael commented Sep 27, 2026 •

Copy link
Copy Markdown

Summary

Fixes #1033 (floating-point/vector remainder).

Fixes #1034 (operator precedence).

I maintain ProjectM TV, an open-source Android TV integration of projectM. I discovered this accidentally while listening to music: ORB - Burnt Ice --- Isosceles edit showed only a tiny white element in the centre, even with bass-heavy music. Tracing its unchanged preset through the evaluator and generated GLSL exposed three linked HLSL translator defects.

This restores floating-point/vector remainder and correct multiplicative precedence, while retaining integer remainder for integer expressions. The diff contains only the compiler changes and regression tests; reproduction presets, images and documentation are linked from a separate evidence branch.

Defects and fix

  • GetBinaryOpResultType forced every % expression to scalar int, discarding float and vector result types. Use the existing arithmetic type-promotion rules.
  • GLSLGenerator cast both operands to scalar int. Emit component-wise mod for float expressions and preserve the promoted integer type for integer %; keep vector dimensions in the legacy GLSL fallback.
  • _binaryOpPriority omitted the % entry, shifting subsequent entries. Add its multiplicative priority and a compile-time check that the table matches the operator enum.

For Burnt Ice, ret = one % two * 2 uses two float3 colour samples. It became scalar integer remainder of one and two * 2, then grayscale output. It now becomes mod(one, two) * 2 with float3 values intact. HLSL supports floating-point remainder; GLSL mod agrees in HLSL %'s defined same-sign domain (Microsoft specification). This does not alter the existing fmod intrinsic mapping.

Real music and real preset evidence

I compared unchanged community presets on old and fixed engines with the same 1926 – Rainy Melody (Original Mix) excerpt, 1:00–1:20, using matched clocks, random seeds, audio and textures.

Real preset Reproduced problem Patched result
ORB - Burnt Ice --- Isosceles edit Tiny white central mark; float3 colours reduced to integers Coloured scene; mean lit coverage 0.00965% → 54.88%
martin - elusive impressions mix2 1-q28%2/4 grouped as (1-q28)%(2/4); mostly flat pale output Correct zoom arithmetic and detailed image
martin - city lights Integer scrolling expression int(...)%8-4 grouped as int(...)%(8-4) Correct centred scrolling offset; subtler visual difference

A staged build on Elusive Impressions separates precedence from result typing: precedence alone still emits integer division (int(q28)%2)/4; adding correct result typing emits division by 4.0 and matches the full fix byte-for-byte in the measured window. Integer-valued q28 makes the remaining operand cast harmless in that particular experiment. Comparison/bitwise priority effects outside these expressions are covered by parser tests, without claiming a separate real-preset reproduction.

Real-music comparisons, videos, original presets, generated GLSL and staged-build measurements. Public videos are muted; the music recording is not included.

Burnt Ice with real music: old engine / fixed engine

Elusive Impressions with real music: old engine / fixed engine

The same float3 expression is confirmed in ten presets under controlled audio. All ten per-preset comparisons and downloadable originals are on a separate evidence branch to keep this code diff focused. Other source-scan matches remain unverified; two additional real presets produced no difference in the selected track excerpt and are recorded as negative results.

Reproduction and validation

  • Download the minimal reproduction preset from the separate evidence branch and load it without music or textures: expected uniform RGB (0.5, 0.4, 0.1), original output black. Burnt Ice and same-frame proof are also available there.
  • Added ten regression tests covering result promotion, scalar/vector mixtures, signed/unsigned integers, multiplicative and bitwise precedence, and generated desktop/ES/legacy GLSL. Eight of the initial nine regressions fail on unmodified upstream and pass with the fix.
  • 157 upstream projectM tests pass via CTest; the pinned 4.1.7 integration passes 107 tests. Positive and negative same-sign float vectors plus six mixed-type GPU cases match expected RGB within one 8-bit quantization step.
  • Android TV debug build succeeds for arm64-v8a and armeabi-v7a. Native engine tests pass; the EGL/GLES overlay test was unavailable on this macOS host. No SHIELD runtime validation is claimed.

Visual measurements come from the patched 4.1.7 integration on Apple M4 Pro / desktop OpenGL 4.1 at 256×144 and 30 fps. Upstream master is independently covered by its full test suite. The evidence records these settings, measurements and preset fingerprints.

@johnneerdael

Copy link
Copy Markdown
Author

Quick comparison of all presets:

all-presets-comparison

@kblaschke kblaschke left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great to see a regression test as well.

HLSLParser is always good for such surprises.

@kblaschke

Copy link
Copy Markdown
Member

I'll also cherry-pick this into the 4.1.x release branch and publish a new stable version next week, as this is definitely an improvement worth delivering.

@kblaschke kblaschke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for fixing this!

I'll wait for the build checks to complete and will merge later today.

@johnneerdael

Copy link
Copy Markdown
Author

@kblaschke just happy to be of assistance, if your willing i wouldn't mind a shout-out to my Android tv version that's open source and better then the paid app if I say so myself ;) https://github.com/johnneerdael/ProjectM-TV

@kblaschke

kblaschke commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

@kblaschke just happy to be of assistance, if your willing i wouldn't mind a shout-out to my Android tv version that's open source and better then the paid app if I say so myself ;) https://github.com/johnneerdael/ProjectM-TV

Oh, no problem: we have a link list in the ReadMe and also our wiki here in this repo. Just make the edits, open a PR and I'll merge them ASAP!

@kblaschke
kblaschke merged commit 0517660 into projectM-visualizer:master Sep 30, 2026
41 checks passed
@johnneerdael

johnneerdael commented Oct 2, 2026 •

Copy link
Copy Markdown
Author

@kblaschke PR is ready for README, I added the seperate link to the paid google tv app under a seperate Google/Android TV section and included my own version (made more sense then keep it all under android), mentioning my contributions to 4.1.8 when its out, if you want something changed just let me know and I will adjust it, on the wiki I updated it. I will probably soon add Milkbeat to the list ;) with only the tiniest knod to Winamp and Milkdrop.... As in itself its a regular offline music player although there is some plugin support ...

logo-milkbeat

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.

[DEV BUG] Missing modulo priority entry breaks HLSL operator precedence [DEV BUG] HLSL remainder loses floating-point values and vector components

3 participants