Skip to content

Conversation

@gero3
Copy link
Contributor

@gero3 gero3 commented Sep 20, 2019

This continues the change from d3a37f3 .
Since that broke following examples:

Simply by making sure that the texture updates, the examples work again.

@Mugen87 Mugen87 added this to the r109 milestone Sep 21, 2019
@Mugen87 Mugen87 merged commit 71e98c9 into mrdoob:dev Sep 21, 2019
@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 21, 2019

Updated builds: c9c2ef9

@mrdoob
Copy link
Owner

mrdoob commented Sep 21, 2019

Good catch! Thanks!

@WestLangley
Copy link
Collaborator

@mrdoob Should we rename to 3DDataTexture and 2DArrayDataTexture for consistency?

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2019

@WestLangley I don't think Javascript allows methods/vars to start with a number. What's why the 3ds loader name is TDSLoader 😕

@WestLangley
Copy link
Collaborator

@mrdoob I was grepping for Texture.js and missed those cases... That is why I mentioned it.

We used to use the webgl convention: TextureCube. We later changed the nomenclature to CubeTexture, so now we are stuck with that pattern...

TDDataTexture? (ugh)

Array2DDataTexture is not too bad.

What do you think?

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2019

How about ArrayDataTexture2D?

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2019

We don't have Texture2D. (Maybe we should...) So ArrayDataTexture I guess makes sense too?

@WestLangley
Copy link
Collaborator

I find having both patterns XXXTexture and TextureYYY confusing. It is hard to remember which one it is. I would prefer to stick to the former.

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2019

ArrayDataTexture then?

@WestLangley
Copy link
Collaborator

Sure. What about DataTexture3D? That is using the TextureYYY pattern.

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2019

I think Data3DTexture.

@WestLangley
Copy link
Collaborator

DataTexture3D -> Data3DTexture

DataTexture2DArray -> ArrayDataTexture

I'll leave it up to you. :-)

@mrdoob
Copy link
Owner

mrdoob commented Sep 24, 2019

Sounds good! I'll add it to my list 🤓

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.

5 participants