Bug 4722 - Multistop gradients only work if the stops are equally spaced and end in offsets 0.0 and 1.0.
Summary: Multistop gradients only work if the stops are equally spaced and end in offs...
Status: RESOLVED FIXED
Alias: None
Product: cairo
Classification: Unclassified
Component: pdf backend (show other bugs)
Version: 1.1.1
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Kristian Høgsberg
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-10-09 08:13 UTC by Jens Taprogge
Modified: 2005-10-10 20:20 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Fix by using PDF's Stiched Functions (Type 3). (6.60 KB, patch)
2005-10-09 08:19 UTC, Jens Taprogge
Details | Splinter Review
Also Fix the comment. (6.60 KB, patch)
2005-10-09 08:28 UTC, Jens Taprogge
Details | Splinter Review
Patch modified according to Kristian's wishes. (6.59 KB, patch)
2005-10-11 10:49 UTC, Jens Taprogge
Details | Splinter Review

Description Jens Taprogge 2005-10-09 08:13:49 UTC
a) Multistop gradients only work if the stop offsets are equally sapced.
b) The first and last stop have to be 0.0 and 1.0 respectively: while gv and
evince can cope with other values, acroread can not.
Comment 1 Jens Taprogge 2005-10-09 08:19:45 UTC
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.
Comment 2 Jens Taprogge 2005-10-09 08:28:20 UTC
Created attachment 3521 [details] [review]
Also Fix the comment.
Comment 3 Kristian Høgsberg 2005-10-10 12:04:49 UTC
(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
Comment 4 Jens Taprogge 2005-10-11 04:26:11 UTC
(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
Comment 5 Kristian Høgsberg 2005-10-11 07:35:53 UTC
(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
Comment 6 Jens Taprogge 2005-10-11 10:49:04 UTC
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
Comment 7 Kristian Høgsberg 2005-10-11 13:20:47 UTC
(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.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.