Add support for external camera feed from external plugin on Android

[elmajime]

This MR reflects the implementation of proposal godotengine/godot-proposals#10678

As indicated in the proposal, these changes are needed for the creation of an ARCore plugin basic feature: camera feed display.

[Chaosus]

CI failed, you need to fix it and squash the commits after that.

[dsnopek]

Here's two previous PRs that have aimed to implement GL_TEXTURE_EXTERNAL_OES:

• Add external texture support #83519
• Add support for external textures on Android #89840

One of the important notes from the discussion on the first PR, is that we already have something in the OpenGL renderer that we're calling an "external texture". To avoid confusion, I think we should rename the old TextureStorage::texture_create_external() to something else, and use the term "external" for GL_TEXTURE_EXTERNAL_OES.

On that PR, I gave the following name suggestions:

Maybe:

• TextureStorage::texture_create_foreign()
• TextureStorage::texture_create_reference() -- the idea being it "references" a texture created elsewhere
• TextureStorage::texture_create_remote()
• TextureStorage::texture_create_third_party()
• TextureStorage::texture_create_independent()

Naming is hard :-)

[dsnopek]

This needs to be rebased on top of PR #96982, now that that has been merged

[dsnopek]

Thanks! I haven't tested, but for the most part the code looks good to me :-)

There's just a handful of changes that I don't think are needed.

[BastiaanOlij]

This is looking really good. Few small nitpicking cleanup things David and myself noted, once those are cleaned up this can be merged IMHO.

[elmajime]

@dsnopek and @BastiaanOlij, just push force the commit to take into account your comments

[BastiaanOlij]

Look good to me!

[dsnopek]

Thanks! The code looks good to me after the latest changes. The PR is currently failing CI due to minor code formatting issues, but once those are resolved and this is passing tests, then I think it's ready to go :-)

[AThousandShips]

And the fix for the comma

[BastiaanOlij]

This also needs to be freed in RasterizerGLES3::finalize

[BastiaanOlij]

One small nitpick fix and I think this is good to merge.

[Repiteo]

The commits will need to be squashed before this can be merged; check out the documentation for a guide

[elmajime]

The commits will need to be squashed before this can be merged; check out the documentation for a guide

@Repiteo my last fork sync with master seams to have messed up my commit history, do you think I need to do anything more than squashing the fixup?

i.e. can the merge stay or do I need to remove that and rebase instead?

[Repiteo]

It should be removed and rebased, yeah. Merging against master is a common beginner's trap for amend/rebase workflows, but it's thankfully one that's relatively easy to resolve. For the sake of clarity: the endgoal is to make the Added external camera feed from external plugin on Android commit the only commit on this PR

[elmajime]

Yes I don't know why this latest sync ended up being a merge when all the previous ones where rebase. I will do it manually for now on. Le mer. 30 oct. 2024, 16:39, Thaddeus Crews ***@***.***> a écrit :

…

It should be removed and rebased, yeah. Merging against master is a common beginner's trap for amend/rebase workflows, but it's thankfully one that's relatively easy to resolve. For the sake of clarity: the endgoal is to make the Added external camera feed from external plugin on Android commit the *only* commit on this PR — Reply to this email directly, view it on GitHub <#96705 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AK6RF3FBZJKSDBGYKFDKIVLZ6D4T5AVCNFSM6AAAAABN22X32CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBXGU4DCMRTHA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[Repiteo]

Thanks! Congratulations on your first contribution! 🎉