EditorPlugin._forward_3d_draw_over_viewport needs to provide an indication of which viewport is being drawn

[blackears]

Describe the project you are working on

An editor plugin which has a feature which displays the length of edges of a mesh.

Describe the problem or limitation you are having in your project

I am using the Camera3D of the viiewport to determine the location of points of my mesh when projected onto the camera's near plane. I can then use this to draw labels on the component displaying useful information such as edge length. Since this relies on knowing what the viewport camera is, I need this information when processing the _forward_3d_draw_over_viewport() call. At the moment, there is no indication which viewport is being drawn so I'm just getting the first 3D viewport provided by EditorInterface. Unfortunately, this means I'm using the wrong camera for the other viewports.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

This method should have an extra parameter added with the index of the 3D viewport being rendered. Or at least pass in the Camera3D of the viewport being drawn.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

The API should be changed to that the index of the viewport being drawn is passed in along with the overlay control.

If this enhancement will not be used often, can it be worked around with a few lines of script?

No

Is there a reason why this should be core and not an add-on in the asset library?

Requires a change to core library.

[AThousandShips]

This method should have an extra parameter added with the index of the 3D viewport being rendered. Or at least pass in the Camera3D of the viewport being drawn.

This would break compatibility, so some other means of accessing it would be required here to avoid that

[AThousandShips]

If this enhancement will not be used often, can it be worked around with a few lines of script?

Yes it should be trivially possible with:

func find_viewport(overlay):
    for i in 4:
        if EditorInterface.get_editor_viewport_3d(i).is_ancestor_of(overlay):
            return i
    return -1

With the argument in _forward_3d_draw_over_viewport

[blackears]

That method is not working for me. I modified the code, and it keeps returning null:

func find_viewport(overlay:Control)->Viewport:
    for i in 3:
        var viewport:Viewport = EditorInterface.get_editor_viewport_3d(i)
        if viewport.is_ancestor_of(overlay):
            return viewport
    return null

[AThousandShips]

Does it work with my correction I did earlier? for i in 4:?

[blackears]

No, it doesn't work with 4 either. I suspect that the viewport component is not an ancestor of the overlay.

[AThousandShips]

Then just do viewport.get_parent().is_ancestor_of(overlay) or viewport.get_parent().get_parent().is_ancestor_of(overlay)

[blackears]

That's not working either.

[blackears]

Okay, well, when I use two get_parents() I do arrive at something that works for viewport 0. However, all the other viewports are also acting as if they are children of viewport 0.

And this code is awkward.

[AThousandShips]

Wasn't meant as a permanent solution but as a potential temporary fix

This might also indicate that the way the overlay is drawn isn't entirely as this proposal expects or that it's drawn in a way that's agnostic to the specific viewport, not entirely sure what the code does in all the cases

But the problem remains: We can't add a new argument to a virtual method, that breaks compatibility in a major way, so it has to be conveyed some other way

[blackears]

Can you add a second method with the extra parameter? And perhaps have the second method call the current method if it isn't overridden?

[AThousandShips]

Possibly, but that sounds pretty hacky and unstable to me

[blackears]

I think this is a vital piece of information that nearly anyone wanting to use this method would be interested in, and probably an oversight in the original design. Having two similar methods may not be ideal, but you can mark the first as deprecated to discourage it's use. Then in a future version you could remove the redundant method.

There doesn't seem to be any other way to determine which viewport is being drawn over.

[AThousandShips]

I don't think it's that critical as no one seems to have been needing this before, but the solution of deprecating a virtual method could be done, don't know that it has ever been before though

I think perhaps a more generally useful solution would be to make sure you can get this via the existing EditorInterface system, by making sure the overlay can be checked with the workaround

[blackears]

I would imagine there are not too many Godot users who want to write 3D viewport overlays for the editor. However, for tools programmers this would be important. And I think that tools programmers would understand that the original spec had an omission and had to evolve, and be glad that this important info was now provided. And you could still support the old spec by just having the editor call both methods when the overlay needed to be updated.

Adding a method to EditorInterface to return the viewport based on the overlay seems a bit hacky to me, but would also solve the problem.