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 opacity reset #7186

Merged
merged 7 commits into from
Nov 29, 2023
Merged

Fix opacity reset #7186

merged 7 commits into from
Nov 29, 2023

Conversation

klakhov
Copy link
Contributor

@klakhov klakhov commented Nov 27, 2023

Motivation and context

Resolved #7176

Opacity keeps resetting after changing frame (even without the masks)
opacity-reset

How has this been tested?

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • [ ] I have updated the documentation accordingly
  • I have added tests to cover my changes
  • I have linked related issues (see GitHub docs)
  • I have increased versions of npm packages if it is necessary
    (cvat-canvas,
    cvat-core,
    cvat-data and
    cvat-ui)

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

cvat-ui/src/reducers/settings-reducer.ts Outdated Show resolved Hide resolved
cvat-ui/src/utils/clamp-opacity.ts Outdated Show resolved Hide resolved
@bsekachev
Copy link
Member

Changelog?

Copy link

codecov bot commented Nov 27, 2023

Codecov Report

Merging #7186 (bbc09d4) into develop (3bf88c5) will decrease coverage by 0.05%.
Report is 5 commits behind head on develop.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7186      +/-   ##
===========================================
- Coverage    81.48%   81.44%   -0.05%     
===========================================
  Files          364      365       +1     
  Lines        39877    39886       +9     
  Branches      3702     3700       -2     
===========================================
- Hits         32494    32484      -10     
- Misses        7383     7402      +19     
Components Coverage Δ
cvat-ui 75.49% <100.00%> (-0.09%) ⬇️
cvat-server 87.02% <ø> (-0.01%) ⬇️

bsekachev
bsekachev previously approved these changes Nov 28, 2023
changelog.d/20231127_123423_klakhov_fix_opacity_reset.md Outdated Show resolved Hide resolved
@bsekachev
Copy link
Member

Now there is one more issue, when I navigate from a frame without masks to frame with masks, opacity keeps on low level and masks are not visible

image

Comment on lines 18 to 23
const withMasks = states
.some((_state: ObjectState): boolean => _state.shapeType === ShapeType.MASK);
const opacity = withMasks || job?.dimension === DimensionType.DIMENSION_3D ?
Math.max(shapes.opacity, ENHANCED_DEFAULT_OPACITY) : shapes.opacity;
const selectedOpacity = withMasks || job?.dimension === DimensionType.DIMENSION_3D ?
Math.max(shapes.selectedOpacity, ENHANCED_DEFAULT_SELECTED_OPACITY) : shapes.selectedOpacity;
Copy link
Member

Choose a reason for hiding this comment

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

As an optimization, I would suggest first to check if current opacity/selected opacity already more or equal than enhanced level, just skip the algorithm. Also checking job?.dimension is quicker, so, it is a thing we should probably do first

@bsekachev bsekachev dismissed their stale review November 28, 2023 08:40

Some comments appeared

@bsekachev bsekachev merged commit 5cad935 into develop Nov 29, 2023
34 checks passed
@klakhov klakhov mentioned this pull request Nov 30, 2023
5 tasks
@bsekachev bsekachev deleted the kl/fix-opacity-reset branch December 5, 2023 12:18
@cvat-bot cvat-bot bot mentioned this pull request Dec 11, 2023
amjadsaadeh pushed a commit to amjadsaadeh/cvat that referenced this pull request Dec 14, 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.

Opacity settings keep resetting to high values
2 participants