Bug 4722

Created attachment 3520 [details] [review]
Fix by using PDF's Stiched Functions (Type 3).

And here is the patch that fixes both problems.

Note that it could be optimized to only output each Type 1 and Type 3 Function
once. Furthermore In case of stiched functions (Type 3) old Type 1 Functions
can also be reused if the required function g is g=f(1-x) of an existing
function f: '\Encode 1 0' needs to be changed to '\Encode 0 1' in that case.

Created attachment 3521 [details] [review]
Also Fix the comment.

(In reply to comment #2)
> Created an attachment (id=3521) [edit]
> Also Fix the comment.

Looks good.  Could you read through the CODING_STYLE document and update your
patch to match those guidelines and to apply to CVS head?

Also, I was thinking that we should just always allocate an array of size
n_steps + 2 cairo_pdf_color_stop_t's so we avoid the reallocation, and I you can
add the pdf object id to the cairo_pdf_color_stop_t struct we avoid allocating
the linear_ids array.

thanks,
Kristian

(In reply to comment #3)
> (In reply to comment #2)
> > Created an attachment (id=3521) [edit] [edit]
> > Also Fix the comment.
> 
> Looks good.  Could you read through the CODING_STYLE document and update your
> patch to match those guidelines and to apply to CVS head?

Sure.

> Also, I was thinking that we should just always allocate an array of size
> n_steps + 2 cairo_pdf_color_stop_t's so we avoid the reallocation, and I you can
> add the pdf object id to the cairo_pdf_color_stop_t struct we avoid allocating
> the linear_ids array.

I had considerred allocating an oversized array as well and decided against it
since I considered a gradient wih stops at 0.0 and 1.0 the by far most common
case.  Same goes for the linear_ids.  If you prefer it the other way, I will be
happy to change it though.

Best Regards
Jens

(In reply to comment #4)
...
> > Also, I was thinking that we should just always allocate an array of size
> > n_steps + 2 cairo_pdf_color_stop_t's so we avoid the reallocation, and I you can
> > add the pdf object id to the cairo_pdf_color_stop_t struct we avoid allocating
> > the linear_ids array.
> 
> I had considerred allocating an oversized array as well and decided against it
> since I considered a gradient wih stops at 0.0 and 1.0 the by far most common
> case.  Same goes for the linear_ids.  If you prefer it the other way, I will be
> happy to change it though.

Yeah, I'd prefer the oversize allocation, even if it's not often used.  We're
freeing the memory immediately after printing out the pdf code, and it's not a
lot of memory.  On the other hand, doing only one malloc means that there's only
one out-of-memory case we need to handle which should make for some simpler code.

thanks,
Kristian

Created attachment 3545 [details] [review]
Patch modified according to Kristian's wishes.

(In reply to comment #5)
> Yeah, I'd prefer the oversize allocation, even if it's not often used.  We're

> freeing the memory immediately after printing out the pdf code, and it's not
a
> lot of memory.  On the other hand, doing only one malloc means that there's
only
> one out-of-memory case we need to handle which should make for some simpler
code.
> 
> thanks,
> Kristian

Ok. Here is a modified version of the patch. I hope I did not overlook any
aspects of the CODING-STYLE.

Best Regards
Jens

(In reply to comment #6)
...
> Ok. Here is a modified version of the patch. I hope I did not overlook any
> aspects of the CODING-STYLE.

Excellent, looks good.  I've committed the patch to cvs, thanks.