New gradients #400

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Owner

pointlessone commented Sep 7, 2012

Note: this is not considered to be finished.

Preamble

I needed more advanced gradients than those implemented in the upstream Prawn. I knew PDF supports pretty advanced graphics but I needed just some diagonal axial gradients and some radial gradients. Neither of those are implemented in Prawn.

So, what's in it?

Axial gradients

Axial gradients are implemented in Prawn now. But the major flaw in the implementation is that you can do only vertical gradients with it. Current implementation is incapable of any other kind of gradients such as diagonal or horizontal. Also it introduces a concept of gradient bounding box which is not present in PDF file format. Also it's not implemented as such in Prawn too, the only thing it does is strikes the rectangle of that box. I think that functionality is out of scope of gradients.

So what changed:

  • Arbitrary axial gradients (e.g. diagonal or horizontal). This is defined by two points: from and to.
  • No misleading bounding boxes
  • New API. More on that a bit later

Radial gradients

This is new to Prawn. Radial gradients can be represented for simplicity as a series of circles. The series is made by moving center of the circle from one point to another while also changing radius of the circle and its color.

In short, they're cool and now you can have them.

New API

Please note that I'm rather new to Prawn. I'm not aware of any API guidelines on this project and seek an advice on how to do it better. I see that Prawn took more functional approach to API. A series of calls to Document object builds actual document. But I have some methods that construct object that are meant to be used in further calls.

Example:

Prawn::Document.generate("test.pdf") do
  g = linear_gradient [0, 0], [100, 100], "ff0000", "0000ff"
  fill_color g
  fill_rectangle [0, 100], 100, 100
end 

I've took this approach because page needs to have a reference to a gradient in order to use it. Also the same gradient can be used multiple times through the document on different pages. I think it's easier to pass only one argument repeatedly than four or six.

I'm open for suggestion if you know how to do it better.

An apology to Wojciech Piekutowski (@wpiekutowski)

Dear @wpiekutowski, I've removed you file because I didn't see how I could save it. After all the edits I've made the only thing that have left after your file was only a copyright notice. I'd like to keep your name but new file have virtually no resemblance of yours. I'm open for suggestion on how to keep your name on the project.

@pointlessone pointlessone New gradients
* Axial gradients (with arbitrary gradient vectors)
* Radial gradients
67762bc
Contributor

wpiekutowski commented Sep 26, 2012

cheba, thanks for improving gradients support. I'm glad Prawn will support more types of gradient. :)

Your change is breaking the existing API. I guess it wasn't the best idea from my side to have fill_gradient and stroke_gradient with a fixed arguments list, but anyway I think it's important to not break the API or at least leave the current methods working for some time with a depreciation warning. Personally I'd stick to fill/stroke_gradient methods with hash options.

I think your LinearGradient class has some resemblance to my gradient code, so it would be nice if you could still mention me in the header. :) This is quite common practice to list all contributors, even if a library was totally refactored. But anyway, that's for the maintainer to decide.

Owner

pointlessone commented Sep 26, 2012

@wpiekutowski I will add your name as the original implementator of gradients in Prawn.

I can add fill/stroke_gradient methods back with deprecation notice and make them work in compatible way. I can't see where hash options can fit here though. All arguments are mandatory and there's no point making it more complicated than needed with hashes.

What do you think new API might look like?

Contributor

wpiekutowski commented Sep 27, 2012

Thanks @cheba.

I think you could make all arguments optional, except for the first one. Then class-check the first one to see if it's a Hash or Array (point). In the latter case you could output a warning and validate all other arguments for presence (raise ArgumentError if missing). I wonder what's @bradediger opinion about depreciation warnings vs breaking API changes.

Member

bradediger commented Sep 27, 2012

At the moment, I'm fine with small breaking API changes in non-critical
parts of the code like this.

On Thu, Sep 27, 2012 at 7:20 AM, Wojciech Piekutowski <
notifications@github.com> wrote:

Thanks @cheba https://github.com/cheba.

I think you could make all arguments optional, except for the first one.
Then class-check the first one to see if it's a Hash or Array (point). In
the latter case you could output a warning and validate all other arguments
for presence (raise ArgumentError if missing). I wonder what's @bradedigerhttps://github.com/bradedigeropinion about depreciation warnings vs breaking API changes.


Reply to this email directly or view it on GitHubhttps://github.com/prawnpdf/prawn/pull/400#issuecomment-8934271.

Owner

pointlessone commented Sep 27, 2012

@bradediger please don't merge this pull request yet. I'll do some changes over the weekend. I will add @wpiekutowski's name to the file. Also I'm trying to come up with more Prawn-ish API (i.e. without additional classes as currently this is the only place you can see them in Prawn API).

Member

bradediger commented Oct 27, 2012

Superseded by #408, closing.

bradediger closed this Oct 27, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment