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

test(): add globalCompositeOperation tests #8271

Merged
merged 11 commits into from
Sep 12, 2022
Merged

test(): add globalCompositeOperation tests #8271

merged 11 commits into from
Sep 12, 2022

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Sep 11, 2022

Add tests based on https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/globalCompositeOperation#operations

canvas-node fails to render some of the operations on my machine... strange

commit e121c47
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sun Sep 11 12:12:40 2022 +0300

    checkout

commit 5ce9d1c
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sun Sep 11 12:09:08 2022 +0300

    fix(): golden generation from browser

    await all async tasks

commit 1672b59
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sun Sep 11 11:45:38 2022 +0300

    fix(): filename is truncated by busboy
@github-actions
Copy link
Contributor

github-actions bot commented Sep 11, 2022

Coverage after merging gco into master will be

82.09%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
HEADER.js56%48.15%0%68.18%13, 17, 17, 17, 17, 17–18, 21, 21–22, 22, 22, 22, 22–23, 26, 28–29, 86, 86, 86
src
   cache.ts97.06%90%100%100%43
   canvas.class.ts93.50%90.36%94.23%95.74%1000, 1019, 1019, 1053, 1086–1087, 1115–1116, 1147, 1155, 1264–1265, 1267–1268, 1288–1289, 1425–1426, 1435, 1439, 481–482, 488, 496, 618–619, 667–668, 730–731, 734, 736, 773–775, 808, 813–814, 838–839, 996, 996–997
   config.ts77.27%66.67%66.67%84.62%133, 140–142, 152–154
   constants.ts100%100%100%100%
   intersection.class.ts100%100%100%100%
   pattern.class.ts92.19%85.71%100%96.30%105, 111, 123, 130, 83
   point.class.ts100%100%100%100%
   shadow.class.ts95.71%90%100%100%144, 193, 8
   static_canvas.class.ts89.93%83.79%96.70%92.52%1094–1095, 1095, 1095–1096, 1214, 1264–1265, 1269, 1302–1303, 1381, 1387, 1391, 1416–1417, 1445–1446, 1478–1479, 1516–1517, 1520, 1522, 1522, 1522, 1522, 1526, 1526, 1526–1528, 1551–1552, 1589–1590, 1593, 1595, 1595, 1595, 1595, 1599, 1599, 1599–1601, 1677, 1677–1678, 1729, 1731, 1731, 1731, 1731, 1731–1732, 1735–1736, 1736, 1736–1737, 1740, 1740, 1740, 1743, 1746, 1752, 1754–1755, 1755, 1755, 1758–1759, 1759, 1759, 1762, 267–268, 270–271, 273–274, 287–288, 290–291, 588, 608, 662–665, 859
src/brushes
   base_brush.class.ts100%100%100%100%
   circle_brush.class.ts3.03%0%0%4%104–105, 107, 109–111, 120, 120, 120, 122, 124, 126–128, 130–133, 141, 150, 152, 30, 35–36, 44–48, 52–56, 63–66, 74–77, 79, 87, 87, 87, 87, 87–88, 90, 90, 90–93, 96
   pattern_brush.class.ts5.26%0%0%8.33%18, 23–26, 28–29, 29, 29–34, 36, 40, 48, 48, 48, 56–58, 58, 58, 65–66, 68–69, 69, 73
   pencil_brush.class.ts91.76%85.42%100%93.46%122–123, 149, 149–151, 268, 272, 277–278, 73–74, 89–90
   spray_brush.class.ts2.30%0%0%3.08%105–107, 109–110, 118, 118, 118, 118, 118–119, 121–122, 129–130, 132, 134–138, 147, 151, 151, 151, 157, 157, 157–160, 162–165, 169–170, 172, 174–177, 180, 187–188, 190, 192–193, 195, 202–203, 205–206, 209, 209, 215, 215, 219, 27–28, 30–32, 32, 32–34, 38, 48, 55, 62, 69, 76, 83, 95–97
src/color
   color.class.ts91.58%84.51%100%94.34%324–325, 329–330, 333–334, 44, 50, 76–77, 77, 79, 79, 79–80, 82–83
   color_map.ts100%100%100%100%
   constants.ts100%100%100%100%
   index.ts100%100%100%100%
   util.ts100%100%100%100%
src/controls
   control.class.ts90.24%81.48%86.67%97.50%213, 289, 289, 331, 342, 8
   controls.actions.ts76.38%69.20%93.75%80.07%148, 150, 150, 150, 152, 154, 20, 20, 20, 262–265, 283, 285, 293–294, 296, 296–297, 299–300, 304, 326, 328, 336–337, 339, 339–340, 342–343, 347, 372–373, 375, 377, 383, 387, 387, 387–388, 388, 388, 390, 390, 390–391, 391, 391, 394, 394, 394–395, 395, 395, 422–423, 425, 427, 433, 437, 437, 437–438, 438, 438, 440, 440, 440–441, 441, 441, 444, 444, 444–445, 445, 445, 45, 466–468, 470, 470, 470–471, 474–477, 479, 479, 479–481, 481, 481–483, 485, 485, 485–486, 488, 488, 488–489, 494, 494, 494–495, 497, 499–501, 525–526, 528–530, 571–573, 746, 8
   controls.render.ts84.95%84.78%100%84.44%17, 20, 30–33, 35–38, 48–49, 66, 69
   default_controls.ts94.12%50%100%100%115, 83
src/filters
   2d_backend.class.ts96.15%83.33%100%100%63
   WebGLProbe.ts61.54%80%57.14%54.55%38–39, 49–53, 66–69, 75
   base_filter.class.ts28.49%31.82%36.36%25.47%100, 107–111, 111, 111–112, 119–120, 120, 120–123, 138, 154, 164–169, 173–174, 174, 174–177, 177, 177, 177, 177–179, 181, 186–187, 192–196, 241–244, 258, 258, 258–259, 261, 277–279, 279, 279, 279, 279–280, 283, 285–286, 288–289, 291–293, 297–298, 300, 304–306, 310, 314, 334, 334, 334–338, 373, 77, 77, 77–78, 78, 78–79, 79, 79–80, 85–88, 88, 88–89, 96–99, 99, 99
   blendcolor_filter.class.ts10.10%4.76%28.57%9.86%118–119, 119, 119–121, 123, 133–134, 137, 139–142,

Comment on lines 140 to 141
canvas.backgroundImage = createExisting();
canvas.add(createNew(operation), createPreview(operation));
Copy link
Member

Choose a reason for hiding this comment

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

this will break as soon as we fix globalCompositeOperation to be group scoped.
We should make the tests already aware of group, so adding all those elements, background, previews and circles in the same group as siblings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please explain

Copy link
Member

@asturur asturur Sep 12, 2022

Choose a reason for hiding this comment

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

global composite operation reacts differently if you are using caching or not.
When a group is cached a circle inside a group will stop its global composite operation effect on the group itself and won't reach the canvas, because the group cache is rendered with the group own value of global composite operation.

This isn't explained in fabricJS is just a side effect of caching.

global composite operation should introduce a rule like we have for shadow.
If a global composite operation is detected, caching is forced on, and the effect of global composite operation is from an object to its container.

This will make things predictable for the developer that uses global composite operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so the tests are aligned to your comment:

If a global composite operation is detected, caching is forced on, and the effect of global composite operation is from an object to its container.

Copy link
Member

Choose a reason for hiding this comment

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

ok then, at worst some bit will change and we will verify why and if should chnage

@asturur
Copy link
Member

asturur commented Sep 11, 2022

Add tests based on https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/globalCompositeOperation#operations

canvas-node fails to render some of the operations on my machine... strange

node-canvas has broken copy operation.
image

I remember i noticed it and wrote tests for it

Other return sligthly different results:

image

@ShaMan123 ShaMan123 requested a review from asturur September 11, 2022 23:01
};

function createExisting(center = new fabric.Point(size.width, size.height).scalarDivide(2)) {
return new fabric.Image(colorSphere(radius * 2), {
Copy link
Member

Choose a reason for hiding this comment

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

i meant a real png :D this is the same as before, just called once.
Can we make this a fixture?

@asturur asturur merged commit 9c68362 into master Sep 12, 2022
@ShaMan123
Copy link
Contributor Author

ShaMan123 commented Sep 12, 2022 via email

@ShaMan123
Copy link
Contributor Author

Shall I make a fixture then?

@asturur asturur deleted the gco branch November 1, 2022 23:38
frankrousseau pushed a commit to cgwire/fabric.js that referenced this pull request Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants