StarlingGraphics / Starling-Extension-Graphics

flash.display.Graphics style extension for the Starling Flash GPU rendering framework
https://github.com/StarlingGraphics/Starling-Extension-Graphics/wiki
MIT License
285 stars 88 forks source link

How should UVs be applied to filled primitives. #103

Open jarrodmoldrich opened 10 years ago

jarrodmoldrich commented 10 years ago

Hi Jonathan (and anyone else watching),

I was refactoring the stroke code to be more streamline, and in doing so ripping out those strange and superfluous 'degenerates' I added in a long while back. Apparently the stroke generation for ellipse etc. were leaving the _fillStyleSet flag set on while using moveTo/lineTo to generate the outline, thus creating a fill over the top of the fill generated from the NGon created previously.

When I fixed that, I noticed the NGon was filled with one colour. I quickly noticed that the UVs were very very large. Following the code:

1) beginTextureFill takes a user supplied uv matrix and scales it by (1/width, 1/height) 2) drawEllipse then creates a uv matrix scaled to (width, height) * fillMatrix from 1) and applys it to the NGon 3) the buildSimpleNGon function would create UVs that were the same as their relative positions (an NGon of radius 50 would have UVs in the range of -50 to 50) 4) applyUVMatrix is called straight after which applies the uv matrix from 2), which, if lucky, has been cancelled out to be identity

It took me a while to figure it all out. It seems a number of separate changes that you have been involved with over a long period of time that have conspired to create confusion about where exactly the UVs are supposed to be manipulated.

I have made a branch where the ellipse fill has UV is centred and scaled to the ellipse. Scaling works from the centre, so passing a UV matrix scaled to 2.0 means the texture should remain centred within the ellipse and the texture only half visible within the ellipse. Due to the inexplicable scaling in 1), which I have kept, translation happens in terms of texture pixels. So a translate of (32,0) on a 64 by 64 texture means that it's shifted half to the right.

Please have a look at fix_uvs_patch on the upstream repository and tell me what you think. If everything seems fine to you, I'll merge it with master.

Cheers!

jonathanrpace commented 10 years ago

Hi Jarrod - looks like you've delved into the scary world of UV's and fills :)

I won't have chance to check this out in any detail anytime soon. But after reading your point about centering the UV's for ellipse fills, I thought it might be worth clarifying that the extension aims to mimic Flash's texturespace.

The 1/width, 1/height keeps the matrix translations in pixel coordinates by taking into account the texture's dimensions.

I'm afraid centering the ellipses UV's will break the desired 1:1 mapping between the api's.

It's worth noting that the primitives that the API uses (such as NGon) don't care about this (by default they build their geometry in a way that makes sense for GPUs) - it's the API that contorts the UV trasformations into pixelspace.

jarrodmoldrich commented 10 years ago

Well, as I mentioned, NGon was no longer generating UVs in normalised UV space. I'm not sure why that change got made.

Well I can make the UVs normalised relative to the top left. If you draw an 32x64 ellipse with a 64x64 texture, the ellipse to map to the left-hand side of the texture. Is this how you would expect it?

IonSwitz commented 10 years ago

I have been making some changes to the NGon, but I don't think I changed the UV mapping. If I did, I apologize.

When it comes to UV mapping, I have no opinion, so I leave that in your capable hands, @jarrodmoldrich and @jonathanrpace :)

jarrodmoldrich commented 10 years ago

Thanks IonSwitz. I just need to know what is consistent with flash style rendering, which I know nothing about.

The git log points to Jonathan, which is why I've asked him specifically about the changes.

jarrodmoldrich commented 10 years ago

Would anyone be able to test out the fix_uvs_patch? 48ce8a0

I'd like to merge it with master, but I don't want to cause a disaster ;)

The difference should be noticeable in 01_Graphics_API_Example where the Marble circular fill should now show instead of grey. The branch features a file called expected_output.png which provides a reference for how it should look.

IonSwitz commented 10 years ago

Unfortunately, I am swamped the upcoming week, going to Berlin for the Quo Vadis / International Games Week conference. I hope you get some assistance :)

jarrodmoldrich commented 10 years ago

No worries, IonSwitz :) Enjoy the conference.