-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Modifiy the way to compute baseline to have a better match between canvas and text layer #12896
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
Conversation
c783f73
to
42e24fb
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/ab05c8e5c258f66/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 2 Live output at: http://3.101.106.178:8877/fcfc1981a9cbef0/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/ab05c8e5c258f66/output.txt Total script time: 26.59 mins
Image differences available at: http://54.67.70.0:8877/ab05c8e5c258f66/reftest-analyzer.html#web=eq.log |
The Windows bot seems down again. Aside from that, while this seems to work nicely for the Tracemonkey file, it seems to regress quite a bit of other files unfortunately, so this needs some more work. Examples of noticeable movement are |
42e24fb
to
8057cb2
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.67.70.0:8877/cd302387c6f4e16/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://3.101.106.178:8877/38528dbb4f1a2b4/output.txt |
So I reboot the windows bot. |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/cd302387c6f4e16/output.txt Total script time: 26.63 mins
Image differences available at: http://54.67.70.0:8877/cd302387c6f4e16/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/38528dbb4f1a2b4/output.txt Total script time: 26.75 mins
Image differences available at: http://3.101.106.178:8877/38528dbb4f1a2b4/reftest-analyzer.html#web=eq.log |
I'm not sure to understand how to interpret some results of unit tests. |
In theory the output of the The relevant test-code is found in Lines 27 to 107 in 6249ef5
Unfortunately we're going to need to somehow debug/fix the underlying issue first, since it's virtually impossible to review this patch if we cannot actually trust the test-suite. |
Now that the issue mentioned above should be fixed, could you rebase this and retrigger the tests to see how this goes now? |
8057cb2
to
221d8c3
Compare
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://3.101.106.178:8877/0aa7a86a5d538bc/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.67.70.0:8877/1bbd7ece8d4728b/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/1bbd7ece8d4728b/output.txt Total script time: 26.88 mins
Image differences available at: http://54.67.70.0:8877/1bbd7ece8d4728b/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/0aa7a86a5d538bc/output.txt Total script time: 28.34 mins
Image differences available at: http://3.101.106.178:8877/0aa7a86a5d538bc/reftest-analyzer.html#web=eq.log |
Looking at the latest test results, it seems to be a pretty even split between test-cases that improve respectively regress with this patch; hence it's unfortunately not really clear (at least to me) that this is an overall improvement as-is. |
Yes, for example |
@timvandermeij I've the impress that overall this is an improvement too even if it's a bit worse for some of them. |
/botio test |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/8751e8aba462300/output.txt Total script time: 29.56 mins
Image differences available at: http://3.101.106.178:8877/8751e8aba462300/reftest-analyzer.html#web=eq.log |
/botio-linux test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/c9a878dbc0da2b8/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/c9a878dbc0da2b8/output.txt Total script time: 23.85 mins
Image differences available at: http://54.67.70.0:8877/c9a878dbc0da2b8/reftest-analyzer.html#web=eq.log |
I have looked at all reference test differences with this version of the patch and I'd say this is an overall improvement. Some files are improved quite dramatically, others have minor improvements and for a few it's a bit worse, but given that we have to rely on a heuristic here I don't think that will ever be perfect, so this will remain to be a trade-off. In this case the majority of cases is an improvements, so let's go with this and we can always tweak it further once more test cases are presented to us. Thank you for putting effort into this! |
/botio makeref |
From: Bot.io (Linux m4)ReceivedCommand cmd_makeref from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/10bb771551af041/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @timvandermeij received. Current queue size: 1 Live output at: http://3.101.106.178:8877/4b96eb3e2509288/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/10bb771551af041/output.txt Total script time: 21.91 mins
|
From: Bot.io (Windows)SuccessFull output at http://3.101.106.178:8877/4b96eb3e2509288/output.txt Total script time: 27.44 mins
|
This closed quite a few issues, some of them mentioned above, so nice work! |
In showing text layer, before the patch:

and after the patch: