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

Stroke doesn't draw properly for all vertices #89

Closed AlBirdie closed 10 years ago

AlBirdie commented 10 years ago

Occasionally I experience a very strange inconsistency with the Stroke class in which strokes between two vertices aren't being drawn.

This screenshot illustrates the issue; bildschirmfoto 2014-01-16 um 10 34 32

The blue line indicates the position of another vertex position. In theory, there should be a horizontal line between the two vertex points left and right from the blue line, since the point at the blue line has the exact same y position as those two points.

The y positions used to draw this stroke is as follows;

5 11 347 486 389 345 345 443 361 234 331 205 221

x is += 67 in case you want to reproduce this issue.

This is how that stroke is being drawn: _lineStroke.addVertex(xDC, yDC, _lineThickness); if(j < endX - 1) _lineStroke.addVertex(int(transformation.xTransform(j+1)), yDC, _lineThickness);

I couldn't find anything wrong on my side. At first I suspected that the y positions were somehow not being calculated properly, but since those are integers it can't be the source of the issue.

jonathanrpace commented 10 years ago

Does the line re-appear if you add a small offset to one of the identical y values (ie 345.1)?

If so - I smell a line gradient == 0 -> divide by zero error somewhere.

IonSwitz commented 10 years ago

Yes, I'm guessing here, in createPolyLine / createPolyLinePreAlloc

var dot:Number = (d0x * d1x + d0y * d1y) / (d0 * d1);

d0 and d1 is the distance between two vertices. If one of them is zero, bad things will happen, there and in other parts of the function.

Adding:

            var d0:Number = Math.sqrt( d0x*d0x + d0y*d0y );
            var d1:Number = Math.sqrt( d1x*d1x + d1y*d1y );

            if ( d0 == 0 )
                d0 = 0.00001;
            if ( d1 == 0 )
                d1 = 0.00001;

Makes a test Stroke draw more or less properly, but it's a very bad solution.

For the time being, I would suggest filtering out any points that are identical to the previous point drawn, outside of the Graphics API, when calling addVertex

It should be solved properly, though, but right now I cant fix it.

This code can be used to test the problem:

var stroke:Stroke = new Stroke();
        stroke.addVertex(100, 100);
        stroke.addVertex(150, 150);
        stroke.addVertex(250, 200);
        stroke.addVertex(250, 200); // identical as the previous vertex. Will cause things to break
        stroke.addVertex(400, 300);
        stroke.addVertex(400, 400);

        addChild(stroke);.
AlBirdie commented 10 years ago

Jonathan, yes when I add an offset to one of the values, the line re-appears.

@IonSwitz Thanks for you input, I'll simply omit vertices with identical values then for now.

IonSwitz commented 10 years ago

I am not sure what is the best way to solve this.

One solution would obviously be that addVertex could just ignore vertices in succession if they are identical, but that might cause other problems, such as: If the user adds 4 vertices, then he would assume that the stroke contains 4 and not (say) 3 vertices.

Another solution would - obviously - be to fix the math to treat this special case better, but one of the performance improvements that I made, with the pre-allocated vector lengths, makes it tricky to just skip vertices in the middle of the poly line.

The fastest method is, obviously, to let the code assume that the vertices are "well formed", to maximize speed, and instead maybe throw an Error if adding vertices at the same point as the previous point, but that might also be cumbersome.

I can't really see a good way to solve this. Please let me know if you have any suggestions.

IonSwitz commented 10 years ago

I have checked in a fix that I believe fixes this issue. Please try it out.

IonSwitz commented 10 years ago

And, wow, yes, this update also fixes a problem I had with the drawGraphicsData API I added to the GraphicsEx and ShapeEx API. Now this code works fully:

flashShape = new flash.display.Shape(); flashShape.graphics.lineStyle(1, 0xFFFFFF); flashShape.graphics.drawCircle(300, 300, 300); flashShape.graphics.drawRoundRect(100, 100, 400, 200, 50, 50);

starlingShapeEx = new starling.display.graphicsEx.ShapeEx(); starlingShapeEx.graphics.drawGraphicsData(flashShape.graphics.readGraphicsData()); addChild(starlingShapeEx);

Basically, this means that a user can take any vector shape from flash (the fills are still a problem, though) and just extract the vector shape information with "readGraphicsData" and drawing it in Starling as geometry with "drawGraphicsData".

This code worked before, but there were bugs in the drawing code that I couldn't figure out. Now it is complete :)

AlBirdie commented 10 years ago

Gosh, can't thank you enough IonSwitz! Fixed! :+1:

IonSwitz commented 10 years ago

Can I be so greedy to ask you (again) for a SWC of the lastest stuff? I think this would be a good point to push a new SWC into the repo as well... It would be really cool of you. Thanks

AlBirdie commented 10 years ago

Ask any time. That really is the least I can do to pay you guys back! SWC is on it's way. :)

IonSwitz commented 10 years ago

Thanks, I have uploaded it now. I hate when there is a discrepancy between code and SWC, so now things feel balanced and good :)

AlBirdie commented 10 years ago

You're welcome. Yes, having a SWC that doesn't match the source code can probably lead to some confusions with people that are using pre-compiled SWCs only.