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 picking bugs #8730

Merged
merged 5 commits into from Apr 2, 2024
Merged

Fix picking bugs #8730

merged 5 commits into from Apr 2, 2024

Conversation

Pessimistress
Copy link
Collaborator

@Pessimistress Pessimistress commented Mar 31, 2024

For #8704

Fixing various issues with DeckPicker

Change List

  • luma v9's picking shader module has a regression fix(shadertools) attribute picking regression luma.gl#2059
  • v8 Framebuffer by default created a depth attachment, this is no longer true in v9. Picking FBOs should explicitly create new depth attachments. @felixpalmer There may be an issue with Framebuffer in other places too.
  • v9 parameter handling: when blend* device parameters are set, they override the blend: false GL parameter. This is unfortunately very hard to debug as device parameter is set deeply inside pipeline.draw. I really hope we can get rid of GL parameters soon...
  • Unit test

@coveralls
Copy link

coveralls commented Mar 31, 2024

Coverage Status

coverage: 90.042% (+0.05%) from 89.988%
when pulling a9eaaca on x/picking-fix
into 9117414 on master.

})
]
colorAttachments: ['rgba32float'],
depthStencilAttachment: 'depth16unorm'
});
this.depthFBO = depthFBO;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we can come up with a better name than depthFBO, I find it a bit confusing

pickParameters.blend = false;
} else if (pickable && operation.includes('draw')) {
Object.assign(pickParameters, PICKING_BLENDING);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: A bit terse. Maybe call them PICKING_BLEND_PARAMETERS?

@Pessimistress Pessimistress merged commit ff75666 into master Apr 2, 2024
4 checks passed
@Pessimistress Pessimistress deleted the x/picking-fix branch April 2, 2024 16:34
Pessimistress added a commit that referenced this pull request Apr 2, 2024
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.

None yet

4 participants