Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add color_curve RGBGFX test #1554

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add color_curve RGBGFX test #1554

wants to merge 2 commits into from

Conversation

Rangi42
Copy link
Contributor

@Rangi42 Rangi42 commented Oct 27, 2024

Follow-up to #1553. Adds a test for -C/--color-curve support, which is breaking somehow on 32-bit MinGW.

color_curve.out.pal result.pal differ: char 1, line 1
00:0000 
< 00:0000: ec1f 0516 0748 0000 ff03 e03b 0c7f 0516  .....H.....;....
< 00:0010: ff03 ec1f e075 0000 ff03 ec1f 0748 0000  .....u.......H..
< 00:0020: ff03 1f57 0c7f e075 ff03 3a77 1f57 0c7f  ...W...u..:w.W..
< 00:0030: 9f02 e075 2e09 0000 9f02 2e09 0748 0000  ...u.........H..
---
00:0000 
> 00:0000: ff03 ec1f e075 0000 ff03 e03b 0c7f 0516  .....u.....;....
> 00:0010: ff03 3a77 1f57 0c7f ff03 1f57 0c7f e075  ..:w.W.....W...u
> 00:0020: 9f02 2e09 0748 0000 9f02 e075 2e09 0000  .....H.....u....
> 00:0030: ff03 ec1f 0748 0000 ec1f 0516 0748 0000  .....H.......H..

(The palette order changes from 0 1 2 3 4 5 6 7 to 2 1 5 4 7 6 3 0.)

@Rangi42 Rangi42 added bug Unexpected behavior / crashes; to be fixed ASAP! tests This affects the test suite rgbgfx This affects RGBGFX labels Oct 27, 2024
@Rangi42 Rangi42 added this to the v0.9.0 milestone Oct 27, 2024
@Rangi42 Rangi42 requested a review from ISSOtm October 27, 2024 15:35
@Rangi42 Rangi42 changed the title Add more RGBGFX tests Add color_curve RGBGFX test Oct 28, 2024
@Rangi42 Rangi42 mentioned this pull request Nov 3, 2024
2 tasks
@Rangi42
Copy link
Contributor Author

Rangi42 commented Nov 23, 2024

Verbose logging (-vvvv) shows mismatched "Rel size" values between 32-bit MinGW and every other platform.

Correct log (most platforms): https://pastebin.com/EffXYRg9
Incorrect log (32-bit MinGW): https://pastebin.com/WFMW4S0X

	/*
	 * Computes the "relative size" of a proto-palette on this palette
	 */
	double relSizeOf(ProtoPalette const &protoPal) const {
		// NOTE: this function must not call `uniqueColors`, or one of its callers will break!
		double relSize = 0.;
		for (uint16_t color : protoPal) {
			auto n = std::count_if(RANGE(*this), [this, &color](ProtoPalAttrs const &attrs) {
				ProtoPalette const &pal = (*_protoPals)[attrs.protoPalIndex];
				return std::find(RANGE(pal), color) != pal.end();
			});
			// NOTE: The paper and the associated code disagree on this: the code has
			// this `1 +`, whereas the paper does not; its lack causes a division by 0
			// if the symbol is not found anywhere, so I'm assuming the paper is wrong.
			relSize += 1. / (1 + n);
		}
		return relSize;
	}

Note that I have previously rewritten this function to not depend on std::count_if or std::find, without affecting either logged output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected behavior / crashes; to be fixed ASAP! rgbgfx This affects RGBGFX tests This affects the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants