Add default params for Viewport method toGlobal?

Heya, I wonder if viewport.toGlobals’ params renderWidth and renderHeight could default to the engine’s render width and height, since that’s what’s very often passed in?

Something like this maybe?

 * @param renderWidth defines the rendering width, defaults to the rendering with of the last created engine
 * @param renderHeight defines the rendering height, defaults to the rendering height of the last created engine

public toGlobal(renderWidth: number = Engine.LastCreatedEngine?.getRenderWidth() ?? 1, renderHeight: number = Engine.LastCreatedEngine?.getRenderHeight() ?? 1) {

Edit, or maybe they should default to undefined instead of 1, like the current implementation, or maybe throw an error or log a warning, if there isn’t a last created engine? :thinking:

I am not sure we should create such kind of dependencies. The Viewport class is fully unrelated to the the engine or almost all Babylon. If you are feeling using it often, it is probably better to create a helper in your code where you would be explicit about the engine you pass in.

On a side note but really less of an issue, I am afraid of us internally using it in the same way and basically creating bugs really long to troubleshoot in multi engine scenarii.

As a general practice, keeping the maths fully abstracted sounds like the best idea.

@Deltakosh any thoughts ?

1 Like

We actually had it like that in the past but we removed it to reduce coupling (and thus improve treeshaking)

2 Likes

Aah, okay, I hand’t thought of those things guys, but no biggie, I’ll remember those params once I call it enough times, LOL (especially once I get code completion working again with WebStorm, but that’s another issue and I think WebStorm’s fault LOL). :+1: :slightly_smiling_face:

2 Likes