Skip to content

Enable vertex colors for OBJs files#1074

Merged
pngwn merged 1 commit into
gradio-app:mainfrom
radames:model3d-enable-vertex-color
Apr 26, 2022
Merged

Enable vertex colors for OBJs files#1074
pngwn merged 1 commit into
gradio-app:mainfrom
radames:model3d-enable-vertex-color

Conversation

@radames
Copy link
Copy Markdown
Contributor

@radames radames commented Apr 24, 2022

Configure Babylon.js OBJ loader to enable vertex colors

Description

Even though embedded vertex colors are not officially part of the OBJ standard, we often encounter OBJs with vertex color. This PR enables this feature via https://doc.babylonjs.com/divingDeeper/importers/oBJ. I've also added an OBJ example to the demo as well as the source.

I'm making it as a draft, as I'm not sure if the way I'm accessing the loaders namespace is the best approach

	import * as BABYLON_LOADERS from "babylonjs-loaders";
	BABYLON_LOADERS.OBJFileLoader.IMPORT_VERTEX_COLORS = true;

configure Babylon.js OBJ loader to enable vertex colors
@abidlabs
Copy link
Copy Markdown
Member

Very cool! @pngwn or @dawoodkhan82, can you advise @radames on the loaders namespace?

@radames if that's the only issue, feel free to open up for review and @pngwn or @dawoodkhan82 can give their suggestions as comments on the PR

@pngwn
Copy link
Copy Markdown
Member

pngwn commented Apr 25, 2022

Will take a look in the morrow if @radames thinks it is ready for review. Also happy to wait longer!

@radames
Copy link
Copy Markdown
Contributor Author

radames commented Apr 25, 2022

Thank you all. In regards of the original PR purpose, I think that's is all for now. However, in the future we might want to setup different properties for different loaders. Maybe there is a better way to organize the setup properties or even expose it to the user?

@pngwn pngwn marked this pull request as ready for review April 26, 2022 11:57
Copy link
Copy Markdown
Member

@pngwn pngwn left a comment

Choose a reason for hiding this comment

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

I'm okay with this approach. The only real alternative is if the loader is available on the BABYLON namespace by virtue of the internal instantiation mechanism. It probably is available there somewhere but this way is more explicit.

In terms of exposing more of these options to the user, the challenge there is that Babylon is an implementation detail and we do not want to bind the API of Gradio's Model3D to the internal Babyl;on interface as swapping to a different implementation would then becvome a breaking change.

We will need to review any such options on a case by case basis and create a generic API that would act as an abstraction over the implementation. until we know what those options might look like, I think we should avoid prematurely abstracting the setting of these options.

@radames If you have any ideas about what kinds of option it would be good to expose then I'd encourage you to open a ticket so we can discuss the options, possible APIs, and appropriate implementation in more detail.

@pngwn pngwn merged commit 9ecb5e8 into gradio-app:main Apr 26, 2022
@pngwn
Copy link
Copy Markdown
Member

pngwn commented Apr 26, 2022

Thanks @radames !

@radames
Copy link
Copy Markdown
Contributor Author

radames commented Apr 26, 2022

In terms of exposing more of these options to the user, the challenge there is that Babylon is an implementation detail and we do not want to bind the API of Gradio's Model3D to the internal Babyl;on interface as swapping to a different implementation would then becvome a breaking change.

Hi @pngwn, Thanks for the review. Yes, I agree with you. I suppose we are covering the general components needs now, and if one needs a specific 3D Model component viewer, the custom components approach should covert it.

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.

3 participants