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

Use of repeat stamp and fill_color results in blank document in Acrobat #200

Closed
jkriss opened this issue Feb 10, 2011 · 15 comments
Closed

Comments

@jkriss
Copy link

jkriss commented Feb 10, 2011

This is a weird one. It appears that fe254de was the culprit. The looks_blank.pdf will render properly in Preview, but looks blank in Acrobat.

require 'rubygems'
require 'bundler'
Bundler.require

Prawn::Document.generate("works_fine.pdf") do

  repeat :all do
    text "Testing", :size => 24, :style => :bold
  end

end

Prawn::Document.generate("looks_blank.pdf") do

  repeat :all do
    text "Testing", :size => 24, :style => :bold
  end

  fill_color '000000'
  # stroke_color '000000' # this breaks it, too

end

I think I've isolated the problem. Patch forthcoming...

@jkriss
Copy link
Author

jkriss commented Feb 10, 2011

I'm not certain my assumptions are correct, but this seems to fix the bug: https://github.com/jkriss/prawn/commit/e1af47fd834fdb16d0d9183c9ab0b97d86440b71

My assumptions may be wrong, though – it may break something else. Tests are still passing, at least.

@jonsgreen
Copy link
Member

Ah yes stamps and repeaters I knew they would kick me in the butt in the end. Thank-you for catching this case that I had overlooked.

I think actually the problem is that the repeater stamp should not be touching the graphic state color space. Ideally stamps might have their own graphic state stack implementation some day.

Anyhow, I think that removing the calls to update_colors goes too far in that you would lose the color context in which the repeater is created. For example, you would get an unexpected color on the text with:


Prawn::Document.generate("looks_blank.pdf") do
  repeat :all do
    text "Testing", :size => 24, :style => :bold
  end
  fill_color '662255'
end

I ended up adding your bug and writing a spec to capture the problem and here is the fix I just committed: practicingruby@11b9b70.

Hopefully I haven't broken anything else in the process.

@jkriss
Copy link
Author

jkriss commented Feb 11, 2011

Fantastic. I figured I was being a bit heavy handed in my fix. Thanks for getting this straightened out so quickly.

@cbillen
Copy link

cbillen commented Feb 12, 2011

The problem still remains if you use repeat(:all, :dynamic => true) as of ca022b6

I do not use fill color in my repeater's code.

@mat813
Copy link

mat813 commented Mar 7, 2011

Hum, I upgraded prawn today, and I do have stamps doing graphic things (logos), and they do need to change the colors otherwise I don't really see what stamps are for.

@jonsgreen
Copy link
Member

I am not sure what you are saying. You should be able to change the color inside the stamp. However, I assumed the default behavior should be that the stamp's graphic state is preserved at creation time and not affected by the graphic state on the page (e.g. a header or footer should not change colors just because the text on the page is a different color).

If you really need the stamp to be influenced by the graphic state of its page environment let me know and I might be able to add an option for that.

This was something I considered when first implementing this feature but I was not sure there was a use case for it. I would be curious what the rules of your application are that require this.

@mat813
Copy link

mat813 commented Mar 7, 2011

Well, hum, I think that :

def current_fill_color=(color)
  return if state.page.in_stamp_stream?
  graphic_state.fill_color = color
end

Says that I can't change the color while in a stamp, no ? And anyway, my stamps are all black and white and :

add_comment("  inside")
fill_color(100, 100, 20, 0)
move_to(75.518, 23.388)

gets translated into :

%  inside
/DeviceRGB cs
0.000 0.000 0.000 scn
120.518 73.388 m

which is just plain wrong, I mean, the color and the colorspace are wrong :-)

@mat813
Copy link

mat813 commented Mar 7, 2011

Just mashed up a small example :

require 'prawn'

pdf = Prawn::Document.new(:margin => [40, 45, 50, 45]) do
  create_stamp("logo") do
    fill_color(100, 100, 20, 0)
    move_to(75.518, 23.388)
  end
  stamp_at('logo', [-18, 692])
end

puts pdf.render

renders :

...
[ ] 0 d
/DeviceRGB cs
0.000 0.000 0.000 scn
120.518 73.388 m
Q
...

Feels wrong to me, no ?

@jonsgreen
Copy link
Member

Yes this does seem to be defective. I will need to look into this but won't have time until later tonight.

@jonsgreen
Copy link
Member

So I think I have a potential immediate fix to the issue here: https://github.com/jonsgreen/prawn/tree/stamp_color_issue_200. If you are savvy enough and willing to try that out to verify that it resolves your issue then I will right some specs around it as well and merge it into master.

Again the ultimate solution might be to have a separate graphic state stack implementation for stamps but that might require some reworking of Stamps themselves and is a bit more than I want to take on at this time.

@mat813
Copy link

mat813 commented Mar 8, 2011

That repaired the stamps in my pdf's :-)
Though, it seems a bit of a cludge, maybe as you say, a enclosing the stamp in a q/Q would be a good thing.

@jonsgreen
Copy link
Member

That already happens. In fact the whole current graphic state of the page gets transferred to the stamp when created just in case which accounts for sort of verbose output at the beginning of each stamp:

q
/DeviceRGB cs
0.000 0.000 0.000 scn
/DeviceRGB CS
0.000 0.000 0.000 SCN
1 w
0 J
0 j
[ ] 0 d

The problem is a bit difficult to explain but basically we added an internal stack that mimics the PDF stack so that we can better determine when to open and close graphic states and generally keep track of the graphic state history like a PDF reader does.

Stamps however should not influence the current page stack when created since they really only affect the PDF if and when they are applied.

I think it might make sense for them to have their own stack but then they might need to become first class Prawn objects like Pages so that they can keep track of their eternal state. Currently they seem to piggy back on the Page Object which seems a bit clunky.
I would be interested in hearing the opinions on this from any of the other Core contributors or folks familiar with Prawn internals.

The defect you uncovered was because we were using the internal graphic state stack to determine the color to be added to the pdf output but when writing a stamp that does not happen so a new requested color was not being delivered.

I am not sure I have the time and courage at the moment to mess with the code anymore given all the defects I have introduced of late.

@astjohn
Copy link
Contributor

astjohn commented Mar 12, 2011

@jonsgreen,

Thanks for your work on this. My colored stamps were also repaired.

@jonsgreen
Copy link
Member

I finally added a couple specs and merged this fix into master.

@mat813
Copy link

mat813 commented Mar 14, 2011

Thanks a lot :-)

yob pushed a commit to yob/prawn that referenced this issue Apr 20, 2011
yob pushed a commit to yob/prawn that referenced this issue Apr 20, 2011
 - Ironically there doesn't seem to be any reason to avoid using the page graphic stack
 - Got rid of several checks for stamp stream
 - Ensures that hanging graphic states get closed in stamps and pages
 - Finally moving resolved bugs to correct folder
This issue was closed.
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

5 participants