Gradients can have multiple stops to blend more than two colors

[mogest]

Another change towards getting SVG support going. From the changelog:

Previously, only two colors could be specified in a gradient: the start
and end colors. This change allows any number of colors to be specified,
along with the position between 0 and 1 as to where they should be displayed.

This change also comes with a change to the format of the fill_gradient
and stroke_gradient methods. You can continue to use the old method
parameters and only specify two colors, or use the new keyword arguments
to specify arbitrary stops.

As a bonus, if you use the new method style, apply_transformations is
set true automatically (see below).

Two things you might be curious about:

1. The original code calls .dup.freeze after calling normalize_color. I can't see any reason to do this, as normalize_color always returns a new array, and nothing modifies it.
2. The original code calls process_color, which has no side effects, and does nothing with the return value.

So in both of these cases I've just stripped that code out.

I wasn't sure about implementing the kwargs for the new version of the calls, but it reads a lot better IMO, especially that you can use it in both axial and radial modes. More than happy to entertain different method signatures if you don't like it!

[mojavelinux]

Wow this is brilliant.

[pointlessone]

Just one comment: array of arrays looks a bit awkward. How about a hash? Like { position => colour }?

[mogest]

@cheba, I was setting it up for my next PR, which will be opacity specified per stop in a [position, color, opacity] tuple. But I agree, it's not pretty. I could support all three: ["color", "color", "color"]; {0 => "color", 1 => "color"}; [[0, "color"], [1, "color"]]. Thoughts?

Looks like I'm gonna have to install jruby too and work out why it doesn't like double splatting kwargs.

[pointlessone]

For opacity you still can use the following format:

{
  0 => ["color", opacity],
  1 => ["color", opacity]
}

I'm still not convinced about array of arrays. Especially that you can easily convert internally if you need:

{ a: 1, b: 2 }.to_a # => [[:a, 1], [:b, 2]]
Hash[ *[[:a, 1], [:b, 2]].flatten ] # => {:a=>1, :b=>2}

[mogest]

Works for me @cheba.

Re the check fail, gemspec says minimum Ruby version 2.0.0, but travis is running 1.9-series jruby. Is it really minimum 2.0?

[pointlessone]

Well… We only support MRI 2.0+ so we don't expect any compatibility with alternative implementation of lower versions.

There's a chance our Travis configuration is not quite right. I'll take a look a bit later.

[packetmonkey]

Sorry about being so slow to get to this, I think we totally want to get this in. I am planing on cutting a new release of prawn later this week / week end and I don't think I'll find time for us to sort this out before then.

So just know we want to get this in and don't read to much into me cutting a release of prawn without merging this first :)

[pointlessone]

@mogest Could you please rebase this PR and take a look at the failing specs?

[mogest]

@cheba rebased.

jruby is failing because its implementation of kwargs is buggy. A *args picks up all the keyword args as well, where it shouldn't. I guess since it's an officially supported version for prawn I'll have to refactor so I don't trip over that somehow. Will attempt soon.

[pointlessone]

@mogest Is this a known issue with JRuby?

[mogest]

The issue jruby/jruby#2799 has a similar class of problem, but they stated in there that they weren't going to do any more work on kwargs in the 1.7 versions, instead asking people to move to 9000. Travis is set up to use 1.7.19.

The tricky part is I'm trying to support the old arguments, as well as the new keyword args. I think if we have to support 1.7 I might have to ditch using the keyword arguments, which would be a shame. Will have a play anyway.

[pointlessone]

@packetmonkey MRI 1.9.3 is not supported (I mean MRI support, not 1.9.3 support in Prawn) for a long time. Only 2.0+ is supported. Shell we probably deprecate JRuby that doesn't support 2.0 mode?

[packetmonkey]

I'm not sure we ever officially posted it anywhere but we only support versions of ruby that the core team supports, from @mogest's post above it looks like the jRuby team isn't supporting anything there than 9000 so tweaking travis and moving forward with that would be in line with that policy.

I however don't know enough about the jRuby ecosystem/use base to know if we would be cutting off a lot of users in a minor release by doing that.

So unless someone tells me that it would be a serious problem for jRuby users, I'm good with moving to just supporting jRuby 9000, we should also make our policy explicit of supporting the latest version of jRuby and all ruby-core supported versions of MRI.

[packetmonkey]

Looking though the changelog as of v 2.0.0 we recommended using 1.7 in 2.0 mode, but that's all I can find, but we did imply our 'support what ruby core' supports.

[pointlessone]

That JRuby recommendation is based solely on performance of JRuby 1.7.18.

Our supported Ruby versions policy mostly matched MRI core team. Whatever is supported by them is supported by us in Prawn. It's only logical to match that for alternative implementations.

We can deprecate pre-9k JRuby support in a patch level release and then drop it completely in a minor release. Especially since we don't need to actually remove or change anything in the already released code. This deprecation would only make it a bit easier to implement new features like this one.

[mojavelinux]

I'm good with moving to just supporting jRuby 9000

+1

we recommended using 1.7 in 2.0 mode

This works until it doesn't. There are numerous problems with 2.0 mode in 1.7, so it's advisable to use with caution and expect strange behavior at times.

[mogest]

Shall we go ahead with deprecating pre-9k JRuby support? It'd be lovely to see this patch go in.

[mogest]

Woohoo, passing build! Thanks for bumping that along, @pointlessone.

How does merging sound?

[mogest]

Just a ping on this PR. I've got people still waiting for gradient support in prawn-svg, and I can't deliver it to them until this PR is merged. I believe it's been reviewed by at least one core member, so should be good to go in as is. Thanks!

[mojavelinux]

👍

[packetmonkey]

@mogest @pointlessone or I am going to check this out this week, depending on who gets to it first (I'm dealing with a lot of day job stuff this week), the code looks good.

Can you confirm if there are/are not any breaking API changes in this?

[mogest]

Thanks @packetmonkey! The API changes are non-breaking; the old arguments are still supported, and the behaviour defaults to the old behaviour when you use the old arguments, too.

[pointlessone]

Thank you @mogest! Well done!

[mogest]

Thanks @pointlessone!