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: Selected object outline has a consistent width with zoom #505

Merged
merged 3 commits into from
Jan 7, 2025

Conversation

ShrimpCryptid
Copy link
Contributor

@ShrimpCryptid ShrimpCryptid commented Dec 24, 2024

Problem

Closes #187, "Outlines are inconsistently thin on high-resolution frames."

We had an issue where the outlines were always 2 pixels wide in the dimensions of the original data. For very high-resolution images, this resulted in very thin outlines that were hard to see.

Outlines are now 2 pixels wide regardless of zoom level or image dimensions!

Estimated review size: tiny, <5 minutes

Solution

  • Edge detection in the fragment shader now steps by onscreen canvas pixels instead of original frame pixels.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Steps to Verify:

  1. Open the preview link and zoom/interact with cells/resize the window: https://allen-cell-animated.github.io/timelapse-colorizer/pr-preview/pr-505/viewer?collection=https%3A%2F%2Fdev-aics-dtp-001.int.allencell.org%2Fassay-dev%2Fcomputational%2Fdata%2Fendothileal_cell_data%2FTimelapse_20x_dataset%2Fcolorizer_outputs%2Fflow_change_movie%2Fcollections_all.json
  2. Compare against the main build: https://timelapse.allencell.org/viewer?collection=https%3A%2F%2Fdev-aics-dtp-001.int.allencell.org%2Fassay-dev%2Fcomputational%2Fdata%2Fendothileal_cell_data%2FTimelapse_20x_dataset%2Fcolorizer_outputs%2Fflow_change_movie%2Fcollections_all.json

Screenshots (optional):

Before After
JohnPaul_20240305_flow_change_endo_cytosol_tubulin_model_combined_cell_model-track_id-000 JohnPaul_20240305_flow_change_endo_cytosol_tubulin_model_combined_cell_model-track_id-000 (4)
JohnPaul_20240305_flow_change_endo_cytosol_tubulin_model_combined_cell_model-track_id-000 (3) JohnPaul_20240305_flow_change_endo_cytosol_tubulin_model_combined_cell_model-track_id-000 (6)

@ShrimpCryptid ShrimpCryptid added the bug Something isn't working label Dec 24, 2024
@ShrimpCryptid ShrimpCryptid self-assigned this Dec 24, 2024
Copy link

github-actions bot commented Dec 24, 2024

PR Preview Action v1.4.8
Preview removed because the pull request was closed.
2025-01-07 17:45 UTC

Copy link

github-actions bot commented Dec 24, 2024

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 75.34% 5313 / 7052
🔵 Statements 75.34% 5313 / 7052
🔵 Functions 58.67% 142 / 242
🔵 Branches 81.19% 449 / 553
File CoverageNo changed files found.
Generated in workflow #1337

@ShrimpCryptid ShrimpCryptid marked this pull request as ready for review December 24, 2024 20:06
@ShrimpCryptid ShrimpCryptid requested a review from a team as a code owner December 24, 2024 20:06
@ShrimpCryptid ShrimpCryptid requested review from interim17 and frasercl and removed request for a team December 24, 2024 20:06
@@ -91,11 +93,10 @@ vec4 getColorRamp(float val) {

bool isEdge(vec2 uv, ivec2 frameDims) {

Choose a reason for hiding this comment

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

is frameDims still needed/used here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, it can be removed!

@ShrimpCryptid ShrimpCryptid merged commit dee7459 into main Jan 7, 2025
3 checks passed
@ShrimpCryptid ShrimpCryptid deleted the feat/outline-uses-screen-pixels branch January 7, 2025 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Outlines are inconsistently thin on high-resolution frames
3 participants