libui-ng / libui-ng

libui-ng: a portable GUI library for C. "libui for the next generation"
https://libui-ng.github.io/libui-ng/
MIT License
581 stars 51 forks source link

fix uiDrawMatrixScale on Linux and macOS #265

Closed matyalatte closed 4 months ago

matyalatte commented 5 months ago

Related to https://github.com/andlabs/libui/issues/331 and https://github.com/libui-ng/libui-ng/pull/54.

This PR removes uiprivScaleCenter to fix the scaling issue. That's a method proposed in the old issue page. (maybe it's the way cody used in the old PR?)

You can see the same result in the tester app on all platforms. windows linux_fixed mac_fixed

And here is another example. I made a function to draw ovals by scaling circles.

uiDrawSave(context);

uiDrawMatrixSetIdentity(&m);
uiDrawMatrixScale(&m, x, y, sx, sy);
uiDrawTransform(context, &m);

path = uiDrawNewPath(uiDrawFillModeWinding);
uiDrawPathNewFigureWithArc(path, x, y, radius, 0, uiPi * 2, 0);
uiDrawPathEnd(path);

uiDrawFill(context, path, &brush);
uiDrawFreePath(path);

uiDrawRestore(context);

It moves circles vertically even if I fix the center points of circles.

https://github.com/libui-ng/libui-ng/assets/69258547/06419a46-52a9-41c6-ab3e-755985559be2

My PR fixes the weird behavior.

https://github.com/libui-ng/libui-ng/assets/69258547/89416ca8-6cd2-46f5-a4c7-c7ce5eaa8796

I also tried to wrap uiArea with uiGrid just to be sure. (That's a workaround proposed by msink in the old PR.) But it did nothing on my machines.

uiGridAppend(grid, uiControl(area), 0, 0, 1, 1, 1, uiAlignFill, 1, uiAlignFill);
matyalatte commented 5 months ago

force pushed because I forgot to apply the latest fix for meson 1.3.0.

matyalatte commented 5 months ago

Added unit tests for uiDrawMatrix. They make sure the uiDrawMatrix* functions store the same values to uiDrawMatrix on all platforms. The master branch doesn't pass them but this PR can pass.

I also updated the version of cmocka because assert_double_equal requires 1.1.6 or later. If I shouldn't update it with this PR, I'll revert the change and rewrite the tests without assert_double_equal.

matyalatte commented 5 months ago

Force pushed because I noticed I should fix the order of multiplication as well. (That's what msink did in the old PR.)

drawMatrixTRS is the test for the order of matrix multiplication.

cody271 commented 5 months ago

If I shouldn't update it with this PR, I'll revert the change and rewrite the tests without assert_double_equal.

Yes updating major dependencies should be a separate branch. Otherwise this looks great. 👍

matyalatte commented 5 months ago

Rewrote assertMatrixEqual with cmocka 1.1.5.

matyalatte commented 5 months ago

Fixed some lines that have whitespaces as indentation.

cody271 commented 4 months ago

Confirmed that this fixes Page 6 / Direct2D: How to Scale an Object rendering on Darwin and Unix.

cody271 commented 4 months ago

I also tried reverting just the fixes to see the new tests fail. Some improvement to the formatting of the output on error:

diff --git a/test/unit/drawmatrix.c b/test/unit/drawmatrix.c
index 28078096..2e263fc0 100644
--- a/test/unit/drawmatrix.c
+++ b/test/unit/drawmatrix.c
@@ -19,14 +19,16 @@ static int compareDouble(double a, double b, double epsilon)
 void cm_print_error(const char * const format, ...);

 // Check if a == b without aborting the test.
-static int expectDoubleEqual(double a, double b, const char* error_prefix)
+static int expectDoubleEqual(double a, double b, int first_error, const char* error_prefix)
 {
+       // Function only used by assertMatrixEqual() macro below right now.
+       const char* func_prefix = first_error ? "(assertMatrixEqual)\n" : "";
        int equal = compareDouble(a, b, EPSILON);
        if (!equal) {
                if (error_prefix == NULL)
-                       cm_print_error("%f != %f\n", a, b);
+                       cm_print_error("%s%f != %f\n", func_prefix, a, b);
                else
-                       cm_print_error("%s%f != %f\n", error_prefix, a, b);
+                       cm_print_error("%s%s%f != %f\n", func_prefix, error_prefix, a, b);
        }
        return equal;
 }
@@ -39,12 +41,12 @@ static int expectDoubleEqual(double a, double b, const char* error_prefix)
 static void _assertMatrixEqual(uiDrawMatrix *a, uiDrawMatrix *b, const char * const file, const int line)
 {
        int equal = 1;
-       equal &= expectDoubleEqual(a->M11, b->M11, "M11: ");
-       equal &= expectDoubleEqual(a->M12, b->M12, "M12: ");
-       equal &= expectDoubleEqual(a->M21, b->M21, "M21: ");
-       equal &= expectDoubleEqual(a->M22, b->M22, "M22: ");
-       equal &= expectDoubleEqual(a->M31, b->M31, "M31: ");
-       equal &= expectDoubleEqual(a->M32, b->M32, "M32: ");
+       equal &= expectDoubleEqual(a->M11, b->M11, equal, "M11: ");
+       equal &= expectDoubleEqual(a->M12, b->M12, equal, "M12: ");
+       equal &= expectDoubleEqual(a->M21, b->M21, equal, "M21: ");
+       equal &= expectDoubleEqual(a->M22, b->M22, equal, "M22: ");
+       equal &= expectDoubleEqual(a->M31, b->M31, equal, "M31: ");
+       equal &= expectDoubleEqual(a->M32, b->M32, equal, "M32: ");
        if (!equal)
                _fail(file, line);
 }

Before:

[==========] Running 6 test(s).
[ RUN      ] (drawMatrixIdentity)
[       OK ] (drawMatrixIdentity)
[ RUN      ] (drawMatrixTranslate)
[       OK ] (drawMatrixTranslate)
[ RUN      ] (drawMatrixScale)
[  ERROR   ] --- M31: 0.125000 != 0.062500
M32: 0.093750 != 0.070312
[   LINE   ] --- ../test/unit/drawmatrix.c:96: error: Failure!
[  FAILED  ] (drawMatrixScale)
[ RUN      ] (drawMatrixRotate)
[       OK ] (drawMatrixRotate)
[ RUN      ] (drawMatrixTRS)
[  ERROR   ] --- M12: 0.050000 != 0.150000
M21: -0.150000 != -0.050000
M31: 0.500000 != 0.802914
M32: 0.250000 != 0.331364
[   LINE   ] --- ../test/unit/drawmatrix.c:138: error: Failure!
[  FAILED  ] (drawMatrixTRS)
[ RUN      ] (drawMatrixMultiply)
[  ERROR   ] --- M31: 0.500000 != 0.395000
M32: 0.250000 != 0.227500
[   LINE   ] --- ../test/unit/drawmatrix.c:172: error: Failure!
[  FAILED  ] (drawMatrixMultiply)
[==========] 6 test(s) run.
[  PASSED  ] 3 test(s).
[  FAILED  ] 3 test(s), listed below:
[  FAILED  ] (drawMatrixScale)
[  FAILED  ] (drawMatrixTRS)
[  FAILED  ] (drawMatrixMultiply)

 3 FAILED TEST(S)

After:

[==========] Running 6 test(s).
[ RUN      ] (drawMatrixIdentity)
[       OK ] (drawMatrixIdentity)
[ RUN      ] (drawMatrixTranslate)
[       OK ] (drawMatrixTranslate)
[ RUN      ] (drawMatrixScale)
[  ERROR   ] --- (assertMatrixEqual)
M31: 0.125000 != 0.062500
M32: 0.093750 != 0.070312
[   LINE   ] --- ../test/unit/drawmatrix.c:98: error: Failure!
[  FAILED  ] (drawMatrixScale)
[ RUN      ] (drawMatrixRotate)
[       OK ] (drawMatrixRotate)
[ RUN      ] (drawMatrixTRS)
[  ERROR   ] --- (assertMatrixEqual)
M12: 0.050000 != 0.150000
M21: -0.150000 != -0.050000
M31: 0.500000 != 0.802914
M32: 0.250000 != 0.331364
[   LINE   ] --- ../test/unit/drawmatrix.c:140: error: Failure!
[  FAILED  ] (drawMatrixTRS)
[ RUN      ] (drawMatrixMultiply)
[  ERROR   ] --- (assertMatrixEqual)
M31: 0.500000 != 0.395000
M32: 0.250000 != 0.227500
[   LINE   ] --- ../test/unit/drawmatrix.c:174: error: Failure!
[  FAILED  ] (drawMatrixMultiply)
[==========] 6 test(s) run.
[  PASSED  ] 3 test(s).
[  FAILED  ] 3 test(s), listed below:
[  FAILED  ] (drawMatrixScale)
[  FAILED  ] (drawMatrixTRS)
[  FAILED  ] (drawMatrixMultiply)

 3 FAILED TEST(S)
matyalatte commented 4 months ago

Applied the suggested changes.