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

Cull labels when not readable #267

Merged
merged 4 commits into from
Oct 24, 2023
Merged

Cull labels when not readable #267

merged 4 commits into from
Oct 24, 2023

Conversation

pleek91
Copy link
Collaborator

@pleek91 pleek91 commented Oct 24, 2023

Description

Makes labels not visible when the viewport is lower than a 0.2 scale. At this scale labels are not readable. Helps increase performance when zoomed out and many labels would be rendered.

@pleek91 pleek91 requested a review from a team as a code owner October 24, 2023 18:48
@netlify
Copy link

netlify bot commented Oct 24, 2023

Deploy Preview for prefect-graphs ready!

Name Link
🔨 Latest commit af7fffd
🔍 Latest deploy log https://app.netlify.com/sites/prefect-graphs/deploys/6538122296e3c80008634597
😎 Deploy Preview https://deploy-preview-267--prefect-graphs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@@ -59,8 +59,8 @@ export async function flowRunContainerFactory(node: RunGraphNode) {
])

nodesContainer.visible = true
nodesContainer.once('rendered', () => cull())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Noticed this wasn't working before. Need to wait until AFTER everything is rendered before calling cull

This made me realize this is highly inefficient to render things and then cull them. But its working for now. It would be much more efficient to not render things initially and then render them if they are visible. Think we could accomplish that by setting renderable = false on nodes when creating them (before adding them as children). Might mess with that later but for now this works well. Just a bad fps drop when you open a sub node with a lot of nodes because they must all be rendered and then once they are culled the fps jumps back up.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that when a flow is first drawn, all nodes are made visible. I guess it does matter for labels in this case though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, its rendered and then culled. Which is my point here. Unless you're getting at something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

Na at first I thought you may be thinking about the nodes, which will all be visible at first anyway. Ignore me, haha.

@@ -59,8 +59,8 @@ export async function flowRunContainerFactory(node: RunGraphNode) {
])

nodesContainer.visible = true
nodesContainer.once('rendered', () => cull())
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that when a flow is first drawn, all nodes are made visible. I guess it does matter for labels in this case though.

@pleek91 pleek91 merged commit c3b5fb3 into main Oct 24, 2023
4 checks passed
@pleek91 pleek91 deleted the cull-text branch October 24, 2023 20:28
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.

2 participants