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

When the texture is not rotated, the rendering fails #4535

Closed
wants to merge 1 commit into from

Conversation

jorpiell
Copy link

Combine the patch that @dwhipps suggested to fix the bug #2737 and #4515

@dwhipps
Copy link
Contributor

dwhipps commented Oct 27, 2016

@jorpiell This is great, but any idea WHY this is true? I feel like it might be similar to the bug @hpinkos found here 564dbb8

@@ -802,6 +802,7 @@ define([
options.textureMatrix = textureMatrix;
options.tangentRotationMatrix = tangentRotationMatrix;
options.size = options.width * options.height;
options.stRotation = stRotation;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this actually used anywhere? I traced through the code visually and didn't see this particular options object including stRotation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it's used to make a textureMatrix that rotates the st coordinates

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry to be dense, but where is that happening? From what I can tell, only constructRectangle and constructExtrudedRectangle get called with this specific options object and neither of them use it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind, I see it now. Sorry for the noise.

@mramato
Copy link
Contributor

mramato commented Oct 31, 2016

@bagnell and @hpinkos can you take a look at this? Obviously the rotation issues are a hot topic so it would be nice if we can track down the root cause and get them fixed.

@dwhipps
Copy link
Contributor

dwhipps commented Nov 7, 2016

@hpinkos I've always been told that the squeaky wheel gets the grease... here is my weekly squeak for this one.

@hpinkos
Copy link
Contributor

hpinkos commented Nov 11, 2016

I didn't forget about this @jorpiell, @dwhipps
I'm not sure if this is the best solution, but I'm going to keep this PR open until I can dig into it a little more.
We have a code sprint coming up the first week of December, I'll definitely look at it then if I don't get to it sooner

@hpinkos
Copy link
Contributor

hpinkos commented Dec 7, 2016

@jorpiell See the fix I submitted here: #4725

@hpinkos hpinkos closed this Dec 7, 2016
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.

4 participants