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

Wrong gradient attributes occasionally being rendered #1048

Closed
jacksop opened this issue Nov 18, 2017 · 5 comments
Closed

Wrong gradient attributes occasionally being rendered #1048

jacksop opened this issue Nov 18, 2017 · 5 comments

Comments

@jacksop
Copy link
Contributor

jacksop commented Nov 18, 2017

I have a document with several small polygon shapes in different colours over many pages. I am experiencing an issue where occasionally a shape is rendered in the wrong colour.

I believe this is caused by collisions in the registry key used to store the gradient as a page resource. The key is created from a digest of the gradient attributes, packed as unsigned 8-bit characters.

Example

This small example illustrates the issue.

Prawn::Document.generate('gradient.pdf') do                                        
  stroke_axis                                                                      
                                                                                   
  # top left solid pink                                                            
  fill_color 'fe00ff'                                                              
  fill_rectangle [150,512], 100, 100                                               
                                                                                   
  # top right gradient pink                                                        
  fill_gradient [256,512], [356,512], 'ffffff', 'fe00ff'                           
  fill_rectangle [256,512], 100, 100                                               
                                                                                   
  # bottom left solid blue                                                         
  fill_color '0000ff'                                                              
  fill_rectangle [150,256], 100, 100                                               
                                                                                   
  # bottom right gradient blue                                                     
  fill_gradient [256,256], [356,256], 'ffffff', '0000ff'                           
  fill_rectangle [256,256], 100, 100                                                                                                                           
end

screen shot 2017-11-18 at 22 54 49

The key for the gradient on the bottom right is identical to the previously generated key for the top right gradient and consequently the rectangle uses the old gradient definition.

Solution

Would it be possible to avoid packing the attributes down to 8-bit values and to also use a SHA2 digest (for a longer key)?

Suggest changing the method in lib/prawn/graphics/patterns.rb from:

def gradient_registry_key(gradient)                                                
  _x1, _y1, x2, y2, transformation = gradient_coordinates(gradient)                
                                                                                   
  key = [                                                                          
    gradient.type.to_s,                                                            
    transformation,                                                                
    x2, y2,                                                                        
    gradient.r1 || -1, gradient.r2 || -1,                                          
    gradient.stops.length,                                                         
    gradient.stops.map { |s| [s.position, s.color] }                               
  ].flatten                                                                        
  Digest::SHA1.hexdigest(key.pack('HC*'))                                             
end

to:

def gradient_registry_key(gradient)                                                
  _x1, _y1, x2, y2, transformation = gradient_coordinates(gradient)                
                                                                                   
  key = [                                                                          
    gradient.type.to_s,                                                            
    transformation,                                                                
    x2, y2,                                                                        
    gradient.r1 || -1, gradient.r2 || -1,                                          
    gradient.stops.length,                                                         
    gradient.stops.map { |s| [s.position, s.color] }                               
  ].flatten                                                                        
  Digest::SHA2.hexdigest key.join(',')                                             
end
@cervinka
Copy link

I have similar problem. Suggested solution from @jacksop works for me.

@pointlessone
Copy link
Member

@jacksop Thank you for reporting this.

Sorry for the delay. I’m a bit busy right now.

A quick check in the code indeed reveals shortcomings of key construction. Hashing function is OK. We don’t really need longer keys. The flaw is in the pack. Just switching to join is enough to eliminate the issue.

As I mentioned, I’m a bit busy right now so I wouldn’t be able to fix this fast. I’d gladly accept a PR though. So if you wish to work on this feel free to take it. I can help with an advice.

@jacksop
Copy link
Contributor Author

jacksop commented Nov 23, 2017

Thanks @pointlessone. I'll put together a PR.

@jacksop
Copy link
Contributor Author

jacksop commented Nov 23, 2017

@pointlessone Do I need specific access in order to push a branch?

@pointlessone
Copy link
Member

@jacksop Yes. Try submitting a PR from your fork.

jacksop added a commit to jacksop/prawn that referenced this issue Nov 23, 2017
Packing gradient attributes down to 8-bit values causes collisions when generating the SHA1 digest.

Concatinate the gradient attributes into a string in order to calculate the digest for the key.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants