Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supported altanative UV mapper on ExtrudeGeometry #1748

Closed
wants to merge 1 commit into from

Conversation

gyuque
Copy link
Contributor

@gyuque gyuque commented Apr 17, 2012

Implementation of Issues #1396
and may have something to do with #1683?

@mrdoob
Copy link
Owner

mrdoob commented Apr 19, 2012

@zz85 I let you decide on this one ^^

@zz85
Copy link
Contributor

zz85 commented Apr 20, 2012

thanks, i'll take a look into this. :)

@zz85
Copy link
Contributor

zz85 commented Apr 23, 2012

testing this with texture = THREE.ImageUtils.loadTexture( 'textures/ash_uvgrid01.jpg' );

the CylinderUVGenerator seems to work pretty well. I'm not too sure about how's WorldUVGenerator supposed to work though...

CylinderUVGenerator

WorldUVGenerator

@alteredq do you remember how UVs were on ExtrudeGeometry?

merged to https://github.com/zz85/three.js/compare/uvs first.

@alteredq
Copy link
Contributor

@zz85 Like this:

exrude uvs

@zz85
Copy link
Contributor

zz85 commented Apr 26, 2012

@alteredq your screenshot is how the UVs are currently which is the same for my previous screenshot.

It seems like either there's a bug to this kind of UV wrapping (many UV values are way above 1 while usually they are a range from 0..1?) or I don't understand what's going on here (the WorldUVGenerator).

@alteredq
Copy link
Contributor

Well, that's the screenshot from the lib version at the time I merged that example (somebody else created it), whatever that means ;)

@zz85
Copy link
Contributor

zz85 commented Apr 26, 2012

i'm looking at lots of interesting history on ExtrudeGeometry back to last year that I never knew existed :P

@mrdoob
Copy link
Owner

mrdoob commented Apr 26, 2012

Is that good or bad? :P

zz85 added a commit to zz85/three.js that referenced this pull request Apr 27, 2012
i'll shall leave THREE.ExtrudeGeometry.WorldUVGenerator as it is right now until someone have a clearer picture of whats going on. closes mrdoob#1748
@zz85
Copy link
Contributor

zz85 commented Apr 27, 2012

at least there are people playing around with the code :)

i think i'll shall leave THREE.ExtrudeGeometry.WorldUVGenerator as it is right now until someone could give a better suggestion.

(sidenote: maybe i shouldn't be rebasing commits - github doesn't close this pull request automatically after merging now)

@mrdoob
Copy link
Owner

mrdoob commented Apr 27, 2012

Yep. It's always better to merge
You keep the credit of the original author too :S

@alteredq
Copy link
Contributor

What does "rebase" actually do?

I always use just fetch + merge, not even pull. I find it better to have separate steps, less chances to mess up something, also it's easier to deal with different branches from other repos in this way, at least in my client.

@zz85
Copy link
Contributor

zz85 commented Apr 27, 2012

for example there are 2 branch with changes simultaneously
mrdoob -> A - B - C - D
alteredq -> E - F - G - H

if alteredq merges mrdoob, result might be something like
A ~ B ~ C ~ D
~ E ~ F ~ G ~ H

if alteredq rebases mrdoob, alteredq will take mrdoob changes, and replay alteredq's changes after mrdoob.
result would be
A - B - C - D - E' - F' - G' - H'

some like rebases because the commit history looks "cleaner". some people prefer a workflow for a dev branch to rebase the master, and a master to merge the dev branch for example. its also probably useful in a central git workflow (rather than distributed like github)

the drawbacks is that rebase modifies the history and its hashes. this is bad when you have pushed changes onto github and now you have to force push your rebased history. so have to be careful using rebase, if history have been already pushed to some where else.

anyways, looks like i'll will just use merge for the three.js now, sorry about the hiccup.

@alteredq
Copy link
Contributor

Thanks for the explanation. This rebasing just feels so alien to me ;)

I'm already completely used to understand everything through this:

https://github.com/mrdoob/three.js/network

@mrdoob
Copy link
Owner

mrdoob commented Apr 27, 2012

It's alien to me too :)
Closing this as it's been, somehow, merged already.

@mrdoob mrdoob closed this Apr 27, 2012
@arcanis
Copy link
Contributor

arcanis commented Feb 20, 2021

It seems a bit late to point this out, but CylinderUVGenerator seem to have been lost in the merge 😄 (unless it's been removed at some point?)

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.

None yet

5 participants