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

wireframe mode #745

Merged
merged 10 commits into from
Feb 15, 2020
Merged

wireframe mode #745

merged 10 commits into from
Feb 15, 2020

Conversation

meetar
Copy link
Member

@meetar meetar commented Jan 27, 2020

This is a naive approach to wireframe rendering, which manually edits the VBOMesh.element_data indices to produce three line segments for every triangle.

Textures and lighting are disabled, but vertex colors are still applied.

PR for discussion – would be cool to have this as a debugging option.

@meetar
Copy link
Member Author

meetar commented Jan 27, 2020

image

@@ -86,7 +87,7 @@ export default class Texture {
}

bind(unit = 0) {
if (!this.valid) {
if (!this.valid || debugSettings.wireframe) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you tell me what this check is about? Shouldn't any textures used by the geometry still be valid, even if they are not being used for rendering?

Copy link
Member

Choose a reason for hiding this comment

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

I'm getting hundreds of these WebGL errors in Chrome with this:

WebGL: INVALID_OPERATION: texImage2D: no texture bound to target
WebGL: INVALID_OPERATION: texParameter: no texture bound to target
[.WebGL-0x7f8b71f16e00]RENDER WARNING: there is no texture bound to the unit 0

I'm not getting any errors with this check removed though, e.g. reverted to original code.

Copy link
Member

Choose a reason for hiding this comment

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

OK I think I see what you were trying to achieve with this: to get textured points to render as quads, instead of wireframes masked against a sprite?

I will try to think of another way to accomplish this, but we can't disable texture binding this low-level.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep that's the goal – though tbh I've never needed to inspect sprite geometry, I just wanted to try this for completeness

Copy link
Member

Choose a reason for hiding this comment

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

OK this is my attempt to make these more visible, think it works alright: da98a78

(texture must be bound to support many other calls in rendering pipeline)
…ion instead)

clarify wireframe mode only applies to triangles with element data, undefined results otherwise (likely GL errors)
@@ -1152,6 +1153,11 @@ export default class Scene {
// Create lighting
createLights() {
this.lights = {};

if (debugSettings.wireframe) {
Light.enabled = false; // disable lighting for wireframe mode
Copy link
Member

Choose a reason for hiding this comment

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

Turns out this is a more succinct way to disable lighting for wireframe mode, rather than having to create a synthetic light.

return new VBOMesh(this.gl, vertex_data, vertex_elements, vertex_layout, options);
return new VBOMesh(this.gl,
vertex_data, vertex_elements, vertex_layout,
{ ...options, wireframe: debugSettings.wireframe }
Copy link
Member

Choose a reason for hiding this comment

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

Moved wireframe option enable here, rather than having it hard-coded inside the VBOMesh, which (ideally) is more generic (though wireframe mode is itself hard-coded to specific data/draw mode format...).

@bcamper
Copy link
Member

bcamper commented Feb 15, 2020

Thought about this a bit more, decided to move wireframe building code to its own file, to isolate from "normal" code and remove any special wireframe logic from VBOMesh entirely. 13f8e10

@bcamper bcamper merged commit f0e7cf6 into master Feb 15, 2020
@bcamper
Copy link
Member

bcamper commented Feb 15, 2020

Thanks @meetar! This is awesome, wish we'd had it ~4 years ago...

@bcamper bcamper deleted the meetar-wireframe branch February 15, 2020 21:38
@bcamper bcamper added this to the v0.20.1 milestone Feb 16, 2020
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