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

fix(): Disable offscreen check for bg and overlay when not needed #8898

Merged
merged 7 commits into from May 8, 2023

Conversation

asturur
Copy link
Member

@asturur asturur commented May 7, 2023

Motivation

closes #8887

the setting to make bg and overlay independent from zoom and panning are not respected by isOffscreen.
There isn't an easy way to solve this, and this one line fix is probably a way to block the bug for now.

We could have pulled the onscreen check outside of the render function but that could have unintended consequences on existing subclasses and also share the responsibility of offscreen check to more players ( canvas and group at least ).

Changes

The function to render backround or overlay will disable the offscreen check if needed and restore it later

Gist

In Action

@github-actions
Copy link
Contributor

github-actions bot commented May 7, 2023

Build Stats

file / KB (diff) bundled minified
fabric 902.341 (0) 304.447 (0)

@github-actions
Copy link
Contributor

github-actions bot commented May 7, 2023

Coverage after merging fix-bgoverlayoffscreen-master into master will be

83.69%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
index.node.ts7.69%100%0%14.29%17, 20, 23, 29, 32, 35
src
   ClassRegistry.ts100%100%100%100%
   Collection.ts94.71%94.64%86.67%97.09%101, 104, 207–208, 233–234
   CommonMethods.ts96.55%87.50%100%100%10
   Intersection.ts100%100%100%100%
   Observable.ts87.23%85.29%84.62%89.36%144–145, 170–171, 39–40, 48, 57, 91, 99
   Pattern.ts92.21%91.89%90%93.33%114, 138, 149, 158, 51
   Point.ts100%100%100%100%
   Shadow.ts98.36%95.65%100%100%178
   cache.ts97.06%90%100%100%57
   config.ts75%66.67%66.67%82.76%130, 138, 138, 138, 138, 138–140, 151–153
   constants.ts100%100%100%100%
   typedefs.ts100%100%100%100%
src/brushes
   BaseBrush.ts100%100%100%100%
   CircleBrush.ts0%0%0%0%100, 102–104, 113, 113, 113, 115, 117, 119–121, 123–126, 133–134, 141, 143, 28–29, 37–41, 45–49, 56–59, 67–71, 73, 81, 81, 81, 81, 81–82, 84, 84, 84–87, 89, 97–98
   PatternBrush.ts97.06%87.50%100%100%21
   PencilBrush.ts91.01%82.35%100%93.75%122–123, 152, 152–154, 176, 176, 276, 280, 285–286, 68–69, 84–85
   SprayBrush.ts0%0%0%0%100–101, 103–104, 112, 112, 112, 112, 112–113, 115–116, 123–124, 126, 128–132, 141, 145–146, 146, 154, 154, 154–157, 159–162, 166–167, 169, 171–174, 177, 184–185, 187, 189–190, 192, 199–200, 202–203, 206, 206, 213, 213, 217, 22–23, 25–27, 27, 27–29, 33, 42, 49, 56, 63, 70, 89–91, 99
src/canvas
   Canvas.ts78.87%77.54%81.67%79.41%1001–1002, 1002, 1002–1004, 1006–1007, 1007, 1007, 1009, 1017, 1017, 1017–1019, 1019, 1019, 1025–1026, 1034–1035, 1035, 1035–1036, 1041, 1043, 1074–1076, 1079–1080, 1084–1085, 1198–1200, 1203–1204, 1277, 1396, 1519, 1589, 162, 187, 297–298, 301–305, 310, 333–334, 339–344, 364, 364, 364–365, 365, 365–366, 37, 374, 379–380, 380, 380–381, 383, 392, 398–399, 399, 399, 41, 442, 450, 454, 454, 454–455, 457, 539–540, 540, 540–541, 547, 547, 547–549, 569, 571, 571, 571–572, 572, 572, 575, 575, 575–576, 579, 588–589, 591–592, 594, 594–595, 597–598, 610–611, 611, 611–612, 614–619, 625, 632, 669, 669, 669, 671, 673–678, 684, 690, 690, 690–691, 693, 696, 701, 714, 742, 742, 800–801, 801, 801–802, 804, 807–808, 808, 808–809, 811–812, 815, 815–817, 820–821, 891, 903, 910, 931, 963, 984–985
   SelectableCanvas.ts94.39%91.16%94.55%96.63%1113, 1113–1114, 1117, 1137, 1137, 1195, 1248–1249, 1270, 1278, 1403, 1405, 1407–1408, 512, 692–693, 695–696, 696, 696, 745–746, 807–808, 861–863, 895, 900–901, 928–929
   StaticCanvas.ts96.85%92.86%100%98.61%1117–1118, 1118, 1118–1119, 1239, 1249, 1303–1304, 1307, 1342–1343, 1420, 1429, 1429, 1433, 1433, 1480–1481, 325–326, 343, 774, 786–787
   TextEditingManager.ts100%100%100%100%
src/color
   Color.ts92.16%86.49%100%94.29%330–331, 335–336, 339–340, 58, 88–89, 89, 91, 91, 91–92, 94–95
   color_map.ts100%100%100%100%
   constants.ts100%100%100%100%
   util.ts100%100%100%100%
src/controls
   Control.ts93.90%88.89%90.91%97.73%235, 322, 322, 357
   changeWidth.ts100%100%100%100%
   commonControls.ts100%100%100%100%
   controlRendering.ts81.63%78%100%84.78%106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
   drag.ts100%100%100%100%
   polyControl.ts5.97%0%0%11.11%100, 105, 119, 119, 119, 119, 119, 121–124, 124, 127, 134, 17, 25–29, 29, 29, 29, 29, 29, 29, 29, 50–56, 56, 56, 56, 56, 58, 63–64, 66, 76, 82–83, 83, 83–84, 88–90, 90, 90, 90, 90, 92
   rotate.ts19.57%12.50%50%21.43%41, 45, 51, 51, 51–52, 55–57, 59, 59, 59, 59, 59–61, 61, 61–63, 65, 65, 65–67, 67, 67–68, 73, 73, 73–74, 76, 78, 80–81
   scale.ts93.57%92.94%100%93.67%129–130, 132–134, 148–149, 181–183, 42
   scaleSkew.ts78.79%64.29%100%85.71%27, 29, 29, 29, 31, 33, 35
   skew.ts91.03%79.31%100%97.67%130–131, 162–163, 170, 176, 178
   util.ts100%100%100%100%
   wrapWithFireEvent.ts100%100%100%100%
   wrapWithFixedAnchor.ts100%100%100%100%
src/env
   browser.ts84.21%77.78%50%100%14, 17
   index.ts100%100%100%100%
   node.ts76.92%50%66.67%88.24%27, 31–32, 32, 32
src/filters
   BaseFilter.ts21.93%22.81%32%19.05%106–109, 109, 109–110, 116, 116, 116–119, 137, 155, 169–174, 178–179, 179, 179–182, 182, 182, 182, 182–184, 190, 199–200,

@asturur asturur requested a review from ShaMan123 May 7, 2023 11:44
Copy link
Contributor

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

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

I think you should fine tune tests and put in the description this regards only rendering

src/canvas/StaticCanvas.ts Show resolved Hide resolved
@asturur
Copy link
Member Author

asturur commented May 8, 2023

I see how the necessity of adding canvas to backgroundImage ( or overlay ) is unhandled.
This will need to be addressed somehow. Can't be left to the dev

Tests do catch regression

@asturur asturur requested a review from ShaMan123 May 8, 2023 10:30
ShaMan123
ShaMan123 previously approved these changes May 8, 2023
@asturur asturur merged commit 72c56bf into master May 8, 2023
16 of 17 checks passed
@asturur asturur deleted the fix-bgoverlayoffscreen-master branch May 8, 2023 23:04
@asturur asturur mentioned this pull request May 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Overlay disappears when panned out of the canvas, even if overlayVpt is set to false
2 participants