fix: LEAP-419: Fix brush issues appeared after fixing zoom performance #1625
Conversation
src/regions/BrushRegion.js
Outdated
| @@ -271,6 +271,11 @@ const Model = types | |||
| const ctx = layer.canvas.context; | |||
|
|
|||
| ctx.save(); | |||
| if (isFF(FF_ZOOM_OPTIM)) { | |||
| ctx.beginPath(); | |||
| ctx.rect(self.parent.alignmentOffset.x, self.parent.alignmentOffset.y, self.parent.stageWidth, self.parent.stageHeight); | |||
There was a problem hiding this comment.
why do we need this rect here?
There was a problem hiding this comment.
also looks like this rect is created with wrong dimensions on zoomed in image — brush is not displayed during drawing after some line, but displayed after you finish it
There was a problem hiding this comment.
It's dynamic clipining of drawing brush region. As long as we have this optimization with rendering strokes on fly we need to duplicate clipping functionality during it or we will be able to have strokes renered out of image bounds till we release mouse button. And without this clipping test will be failed at https://github.com/HumanSignal/label-studio-frontend/pull/1625/files#diff-1d9ae97357e362d1cf5b1ed46609b84b9e7cdfd80a8158e8f4c955d0187628e1R64 and https://github.com/HumanSignal/label-studio-frontend/pull/1625/files#diff-1d9ae97357e362d1cf5b1ed46609b84b9e7cdfd80a8158e8f4c955d0187628e1R90
| @@ -648,11 +648,25 @@ const Model = types.model({ | |||
| return { | |||
| views: { | |||
| getSkipInteractions() { | |||
| const manager = self.getToolsManager(); | |||
| if (isFF(FF_ZOOM_OPTIM)) { | |||
| if (skipInteractions) return true; | |||
There was a problem hiding this comment.
one more reason to rewrite this whole approach: skipInteractions is just a var, so it won't invalidate this getter per se, so it might loose some checks.
| const tool = item.parent.getToolsManager().findSelectedTool(); | ||
| const isMoveTool = tool && getType(tool).name === 'MoveTool'; | ||
|
|
||
| if (tool && !isMoveTool) return; |
There was a problem hiding this comment.
looks like this fixes bug in both states — with flag ON and OFF. but how?
is there some fix for FF ON down the code?
There was a problem hiding this comment.
This is how it worked before I had found some problems with this part. Right now the solution is here
https://github.com/HumanSignal/label-studio-frontend/pull/1625/files#diff-fe7c98bf1c96a7516510c192b4c2d947adaef36b394240756f6d044b32eb2cc8R652-R662
and it looks better for me.
Gondragos
force-pushed
the
fb-leap-419/brush-fixes
branch
from
7326961 to
f0cd2d7
Compare
Gondragos
force-pushed
the
fb-leap-419/brush-fixes
branch
from
f0cd2d7 to
2468827
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1625 +/- ##
==========================================
- Coverage 68.17% 64.56% -3.62%
==========================================
Files 443 443
Lines 28662 28688 +26
Branches 7615 7518 -97
==========================================
- Hits 19540 18521 -1019
- Misses 7865 10167 +2302
+ Partials 1257 0 -1257 ☔ View full report in Codecov by Sentry. |
HumanSignal#1625) * fix: LEAP-419: Fix brush issues appeared after fixing zoom performance * Add tests * Fix clipping boundaries * Fix tests * Fix missprints * Add debugging info * Fix tests according to CI problems * Update ls-test * Fix tests about drawing without ctrl * Update ls-test
PR fulfills these requirements
[fix|feat|ci|chore|doc]: TICKET-ID: Short description of change madeex.fix: DEV-XXXX: Removed inconsistent code usage causing intermittent errorsChange has impacts in these area(s)
(check all that apply)
Describe the reason for change
Fixes after #1565 for LEAP-32
What does this fix?
What libraries were added/updated?
@heartexlabs/ls-testDoes this change affect performance?
Nope
Does this change affect security?
Nope
What alternative approaches were there?
N/A
What feature flags were used to cover this change?
fflag_fix_front_leap_32_zoom_perf_190923_shortDoes this PR introduce a breaking change?
(check only one)
What level of testing was included in the change?
(check all that apply)
Which logical domain(s) does this change affect?
Brush,Magic wand,Image Segmentation