Skip to content

Add visual regression test for leading tabs in text #8534

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.

Sign up for GitHub

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

Open
Omar-2718 wants to merge 1 commit into
base: dev-2.0
Choose a base branch
from
+11 −1
Open

Add visual regression test for leading tabs in text #8534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion test/unit/visual/cases/typography.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,13 @@ visualSuite('Typography', function () {
p5.text('p5*js', -p5.width / 2, -p5.height / 2 + 10, p5.width);
screenshot();
});

visualTest('Preserve leading tabs',function (p5, screenshot){
p5.createCanvas(100, 100);
p5.textSize(35);
p5.text('test\n\ttest', 0, 0, p5.width, p5.height);
Copy link
Contributor

@davepagurek davepagurek Feb 18, 2026

Choose a reason for hiding this comment

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

Hi! This wasn't generated with @dhowe's changes in, right? I'm wondering why the screenshot shows the tab having an effect, when it's the buggy code still generating the visual. @Omar-2718 in your browser this was still showing incorrect behaviour right? If you mirror that test more closely here (e.g. with a space before the \n? not sure if that's significant) do you see the screenshot starting to show the incorrect behaviour?

In general I'd expect to see the screenshot show incorrect results, and then only after @dhowe's code is merged, would we be able to lock in the correct behaviour in a screenshot.

Copy link
Author

@Omar-2718 Omar-2718 Feb 18, 2026

Choose a reason for hiding this comment

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

I used @dhowe's code to generate the screenshot (as that is the supposed correct answer for the test case) so that my version (the one that isnt working) fails that test. My bad,
So I'm supposed to push the screenshot of my failing code(it fails with 'test\n\ttest' ) before @dhowe's changes, right? And only after @dhowe's pull is merged, update the screenshot

Copy link
Contributor

@davepagurek davepagurek Feb 18, 2026

Choose a reason for hiding this comment

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

ah ok, that makes sense then! although I'm also curious why CI says that the tests are paying even though this PR doesn't include the fixes yet... in any case, we should merge this after the fix is in.

Copy link
Author

@Omar-2718 Omar-2718 Feb 20, 2026

Choose a reason for hiding this comment

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

image Yeah thats weird, this image is from the CI visual-test-report and somehow passes. When I run it locally, it fails normally. I'm not sure if this behaviour is because of font differences between Ubuntu on the CI and Windows that generated the test image. Or the difference threshold to make the test pass is too big. I can look into it

screenshot();
});
});

visualSuite('textWeight', function () {
Expand Down Expand Up @@ -166,7 +173,7 @@ visualSuite('Typography', function () {
Inter: 'https://fonts.gstatic.com/s/inter/v20/UcCO3FwrK3iLTeHuS_nVMrMxCp50SjIw2boKoduKmMEVuLyfMZhrib2Bg-4.ttf',
Raleway: 'https://fonts.gstatic.com/s/raleway/v36/1Ptxg8zYS_SKggPN4iEgvnHyvveLxVvaooCPNLA3JC9c.ttf',
'Inknut Antiqua': 'https://fonts.gstatic.com/s/inknutantiqua/v16/Y4GRYax7VC4ot_qNB4nYpBdaKU2_xbj5bBoIYJNf.ttf',
Oswald: 'https://fonts.gstatic.com/s/oswald/v57/TK3_WkUHHAIjg75cFRf3bXL8LICs1_FvgUFoZAaRliE.ttf',
Oswald: 'https://fonts.gstatic.com/s/oswald/v57/TK3_WkUHHAIjg75cFRf3bXL8LICs1_FvgUFoZAaRliE.ttf'
};
for (const mode of ['2d', 'webgl']) {
for (const fontName in fonts) {
Expand Down
Binary file added test/unit/visual/screenshots/Typography/textFont/Preserve leading tabs/000.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions test/unit/visual/screenshots/Typography/textFont/Preserve leading tabs/metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"numScreenshots": 1
}
Loading