RenderLine(s) API changes

[mvaligursky]

I'm working on line rendering changes here #3019 and would like to change the API to make it simpler and remove the layers of deprecated functionality from current API.

Current line render API:

// single line with one or two colors
Application.renderLine(start, end, color, [endColor], [options])

// multiple lines, using single color or an array of colors
Application.renderLines(positions, color/colors, [options]);

    options:
    - layer
    - depthTest

Proposed new line rendering API:

changes:

• drawLine / drawLines don't have polymorphic arguments (color is either a single, or an array, not either)
  drawLine draws a single line with a single color
  if more lines or more colors are needed, drawLines needs to be used
• don't use options, but pass those directly to avoid class allocation overhead

// single line with a single color
Application.renderLine(start, end, color = WHITE, depthTest = true, layer = immediate)

// multiple lines with multiple colors
Application.renderLines(positions, colors, depthTest = true, layer = immediate)

Additional convenience function:

// Draw lines forming a transformed unit-sized cube (useful for drawing bounding boxes using a world matrix)
Application.renderWireCube(matrix, color  = WHITE, depthTest = true, layer = immediate)

// Draw lines forming an AABB
Application.renderWireAlignedBox(minPoint, maxPoint, color = WHITE, depthTest = true, layer = immediate)

// Draw lines forming a sphere
Application.renderWireSphere(center, radius, color = WHITE, segments = 20, depthTest = true, layer = immediate)

[LeXXik]

I like it! Especially the convenience methods. I don't really use line rendering outside of debugging purposes, though.
One thing is the renderWireCube accepting a matrix. So, conceptually we are drawing a cube around a matrix? I feel like intuitively it should take something else, like a render component or a mesh, around which we want to draw it.

[mvaligursky]

well the matrix is a representation of the transformation of a unit box. We could pass the entity / graph node instead, but it would limit it to that case. passing entity.getWorldTransform() is almost just as convenient and opens up other sources of matrices (for example bones - even those are graph nodes anyways)

[mvaligursky]

I guess it's similar reason to why I pass min and max points to renderWireAlignedBox and center and radius to render a sphere, instead of BoundingBox and BoundingSphere. It uses lower level data, often allowing you to avoid allocating the bounding volumes, but easily use them as well.

[LeXXik]

Yes, I agree, and I do understand the reasons it is the way it is. It just something that is not immediately clear, e.g. what is matrix and how do I call lady in red get it. Given there will be docs and an example of how it is used, there shouldn't be any questions.

[slimbuck]

Looks good. Can Application.drawLine keep an endColor param?

[mvaligursky]

It could be:

Application.drawLine(start, end, startColor = WHITE, endColor = WHITE, depthTest = true, layer = immediate)

but then you'd always need to pass two colors, which is not common.

People can use this instead in that case?

Application.drawLines([p0, p1], [c0, c1]);

[slimbuck]

Yeah I guess that's a fine compromise.

[willeastcott]

A few comments:

• Rendering wire cubes/sphere (and other shapes) is a utility function and not, IMHO, core functionality. It's worth considering keeping that kind of functionality in external scripts (extras or scripts).
• Should we allow line loops/strips as well a discreet lines?
• Would it be useful to draw a line without having to create Vec3 instances? e.g. Just support x1, y1, z1, x2, y2, z2?
• We're ending up with drawX and renderX in the API. Are we happy with this split?

[LeXXik]

Regarding the loops, I think they are already supported by the current API. A user has an option to provide one additional pair of points to close the loop, if one wants to. Not sure how useful the striped lines would be, considering these lines are mostly used for debugging.

[mvaligursky]

It's worth considering keeping that kind of functionality in external scripts (extras or scripts).

I think it's a good idea and even in this case could save enough code size to be worth it. I'll move it to /extras/debug folder .. so that would be all those wire primitives, but also debug texture / depth texture rendering.

Would it be useful to draw a line without having to create

yep, I refactored internals to allow for this .. so I guess I'll add those as well

We're ending up with drawX and renderX

sorry, my mistake .. all those are renderX .. and existing drawX functionality will be implemented using renderX internally and deprecated.

[mvaligursky]

ok for a test I added it to extras (just locally, not in PR), and this is how I'd expect it to be used as an example

var debug = pcx.Debug.get(app);
debug.renderWireAlignedBox(p0, p1, pc.WHITE);

I added static Debug.get function so that this can be easily used in many files without allocating debug and then passing it around the scripts. It's kind of a singleton, but per application. So in any script you can simply call Debug.get(app) and then use it to render debug things.

Thoughts on this @willeastcott ?

[mvaligursky]

Another option, which I like and will test bit more, could be to initialize it by calling to pcx.Debug.init(app);, which would add those rendering functions to the application. So then app. renderWireAlignedBox can be called.

I like that this can be done to more modules .. we could move Lightmapper out of the engine .. and use this way to add it to the engine, without breaking existing API

We could then have a bunch if tickboxes in Editor, and that would generate script to call init functions on those modules to integrate it to the engine. Something to experiment with.