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

upgrade MarlinFX 0.9.2 #96

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
2 participants
@bourgesl
Copy link
Contributor

commented Jun 8, 2018

Here is the PR providing MarlinFX 0.9.2 to run it against CI (testing).

Formal review will happen on webrev (openjfx RFR) as this patch is large (5.8K LOC) but 95% identical to Marlin2D (integrated in OpenJDK 11).

@bourgesl bourgesl requested a review from kevinrushforth Jun 8, 2018

set(points[0], points[1],
points[2], points[3],
points[4], points[5]);
return;
default:
throw new InternalError("Curves can only be cubic or quadratic");

This comment has been minimized.

Copy link
@johanvos

johanvos Jul 4, 2018

Collaborator

Error behavior is changed: before there was a hard check on number of points, and an InternalError thrown when e.g. type = 5. That error won't be thrown anymore.

This comment has been minimized.

Copy link
@bourgesl

bourgesl Jul 4, 2018

Author Contributor

Good catch but this code is always called with type = 4/6... so I simplified.

set(points[0], points[1],
points[2], points[3],
points[4], points[5]);
return;
default:
throw new InternalError("Curves can only be cubic or quadratic");

This comment has been minimized.

Copy link
@johanvos

johanvos Jul 4, 2018

Collaborator

same remark as with Curve, different Error behavior. Probably no issue though.


static final boolean DO_TRACE_PATH = false;

static final boolean DO_CLIP = MarlinProperties.isDoClip();
static final boolean DO_CLIP_FILL = true;

This comment has been minimized.

Copy link
@johanvos

johanvos Jul 4, 2018

Collaborator

This was there before as well, but what is the purpose? It seems to be only used on a single place. Should this be a boolean that can be set via a property?

This comment has been minimized.

Copy link
@bourgesl

bourgesl Jul 4, 2018

Author Contributor

I like constants that are inlined well by hotspot. Moreover the clip fill flag helped me in Marlin 0.8 to handle properly the even/odd rule.
It could be removed but I think it is more obvious like this.


static final boolean DO_TRACE_PATH = false;

static final boolean DO_CLIP = MarlinProperties.isDoClip();
static final boolean DO_CLIP_FILL = true;

This comment has been minimized.

Copy link
@johanvos

johanvos Jul 4, 2018

Collaborator

see above -- there are a number of those.

@bourgesl

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2018

Will push directly in jfx-dev asap

@bourgesl bourgesl closed this Jul 5, 2018

@bourgesl bourgesl deleted the bourgesl:marlin_092 branch Sep 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.