Skip to content

Conversation

@repalash
Copy link

Changes in: OrbitControls, GLTFExporter, GLTFWriter, ObjectLoader, Material, Matrix3, Matrix4, WebGLMultipleRenderTargets

Why

Add missing properties and functions.

What

  • Extend OrbitControls from EventDispatcher like in three.js

  • Put GLTFWriter in GLTFExporter.Utils like in three.js

  • Add functions to GLTFWriter

  • Add name property in GLTFLoaderPlugin

  • Add missing parse functions to GLTFLoader and fix return types of others.

  • Add onBeforeRender in Material

  • Add readonly props isMatrix3 and isMatrix4

  • Add samples to MultisampleRenderTargets

  • Checked the target branch (current goes master, next goes dev)

  • Added myself to contributors table

  • Ready to be merged

@repalash repalash changed the title Add missing functions and properties in Add missing functions and properties in several classes May 25, 2023
Copy link
Contributor

@Methuselah96 Methuselah96 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I did not have time to review the whole thing, if there are parts you would like to get merged in sooner you could consider splitting this into smaller PRs since some of these changes are easier to verify than others.

Comment on lines +90 to +92
static Utils: {
GLTFWriter: typeof GLTFWriter;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

I see Utils being set on GLTFExporter but I'm having trouble seeing where GLTFWriter is declared on it. Can you show where that happens?

Copy link
Author

Choose a reason for hiding this comment

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

Oh, my bad, this was a change I was making a PR for three.js, forgot that its not there.
I will remove this change.

@repalash
Copy link
Author

repalash commented Jun 6, 2023

Hi @Methuselah96, No hurry, please take your time.

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.

2 participants