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 pxToBp and bpToPx calculations when there are many displayed regions #3086

Merged
merged 5 commits into from
Jul 12, 2022

Conversation

cmdcolin
Copy link
Collaborator

I was seeing some small continued inaccuracies in rubberband zooming after looking at #3085

I found that the pxToBp function itself was inaccurate though by hovering over the scalebar and seeing it reporting the wrong coordinate. This adds a fix specifically by looking at all the staticblocks to account for interregionpaddingblocks rather than just interregionpaddingblocks that were on the screen (the previous technique, which notably was not conceptually accurate or implemented correctly)

This pxToBp fix aids proper zooming because the pxToBp coordinates left/right offsets are passed to the rubberband zoom function (moveTo(leftOffset,rightOffset)), so improving pxToBp fixes rubberband zoom also. Has improvements that also flow to other routines that use bpToPx and pxToBp (e.g. synteny uses bpToPx to plot a specific polygon, in bp coordinates, to screen px coordinates)

before, hovering over 4MB shows ~4.7MB
Screenshot from 2022-07-12 00-45-55

after, hovering over 4MB shows ~4MB
Screenshot from 2022-07-12 00-45-34

Apply to more areas

More fixes
@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Jul 12, 2022
@cmdcolin cmdcolin added bug Something isn't working and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) labels Jul 12, 2022
@cmdcolin cmdcolin marked this pull request as draft July 12, 2022 07:10
@cmdcolin cmdcolin marked this pull request as ready for review July 12, 2022 12:20
@codecov
Copy link

codecov bot commented Jul 12, 2022

Codecov Report

Merging #3086 (04f9fbf) into main (68843ef) will increase coverage by 0.04%.
The diff coverage is 90.75%.

❗ Current head 04f9fbf differs from pull request most recent head 92b2326. Consider uploading reports for the commit 92b2326 to get more accurate results

@@            Coverage Diff             @@
##             main    #3086      +/-   ##
==========================================
+ Coverage   61.34%   61.38%   +0.04%     
==========================================
  Files         594      595       +1     
  Lines       27282    27216      -66     
  Branches     6601     6560      -41     
==========================================
- Hits        16735    16706      -29     
+ Misses      10249    10213      -36     
+ Partials      298      297       -1     
Impacted Files Coverage Δ
packages/core/util/Base1DViewModel.ts 90.90% <66.66%> (+18.68%) ⬆️
.../linear-genome-view/src/LinearGenomeView/index.tsx 83.30% <89.01%> (-0.98%) ⬇️
packages/core/util/Base1DUtils.ts 95.00% <95.00%> (ø)
packages/core/util/index.ts 85.96% <100.00%> (-0.26%) ⬇️
...otplot-view/src/DotplotRenderer/DotplotRenderer.ts 77.95% <100.00%> (ø)
...s/dotplot-view/src/DotplotView/components/Axes.tsx 87.69% <100.00%> (ø)
...tenyRenderer/components/LinearSyntenyRendering.tsx 53.38% <100.00%> (ø)
products/jbrowse-web/src/util.ts 27.27% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 68843ef...92b2326. Read the comment docs.

@cmdcolin cmdcolin changed the title Fixes to pxToBp and bpToPx calculations Fix pxToBp and bpToPx calculations when there are many displayed regions Jul 12, 2022
@cmdcolin cmdcolin merged commit 1a626fb into main Jul 12, 2022
@cmdcolin cmdcolin deleted the improve_pxtobp branch July 12, 2022 18:05
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.

None yet

1 participant