Skip to content

Commit

Permalink
Raise error when zero length is provided to dash
Browse files Browse the repository at this point in the history
  • Loading branch information
practicingruby committed Oct 12, 2014
1 parent 371ef9d commit e65c244
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 0 deletions.
5 changes: 5 additions & 0 deletions lib/prawn/graphics/dash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ module Dash
def dash(length=nil, options={})
return current_dash_state if length.nil?

if length == 0 || length.kind_of?(Array) && length.any? { |e| e == 0 }
raise ArgumentError,
"Zero length dashes are invalid. Call #undash to disable dashes."
end

self.current_dash_state = { :dash => length,
:space => length.kind_of?(Array) ? nil : options[:space] || length,
:phase => options[:phase] || 0 }
Expand Down
8 changes: 8 additions & 0 deletions spec/graphics_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,14 @@
@pdf.graphic_state.dash[:dash].should == 5
end

it "should raise an error when dash is called w. a zero length or space" do
expect { @pdf.dash(0) }.to raise_error(ArgumentError)
expect { @pdf.dash([0]) }.to raise_error(ArgumentError)
expect { @pdf.dash([0,0]) }.to raise_error(ArgumentError)
expect { @pdf.dash([0,0,0,1]) }.to raise_error(ArgumentError)
end


it "the current graphic state should keep track of previous unchanged settings" do
@pdf.stroke_color '000000'
@pdf.save_graphics_state
Expand Down

13 comments on commit e65c244

@Nowass
Copy link

@Nowass Nowass commented on e65c244 Dec 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
What is the reason for the "zero length" restriction? Currently, I have a problem to render SVG files with dotted lines, because they are represented as a sequence of lines with zero length. My understanding is that line with zero length should be represented as a single point in SVG. (this understanding can be wrong ... of course)

When I comment out this small if case, SVG generation works well. Do you think, there could be some switch, which would disable this restriction in case of SVG for example?

Thanks and have a nice day.
Petr

@pointlessone
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nowass I can't tell for sure as I'm not the author but it appears that 0 dash length is indeed invalid.

Here's a relevant part from PDF Spec [1]:

The dash array’s elements are numbers that specify the lengths of alternating dashes and gaps; the numbers must be nonnegative and not all zero.

Basically, Line Dash Pattern array specifies advances. All 0 values would provide no advancement effectively not letting to draw the line.


[1]: PDF Reference 1.7, Section 4.3.2: Details of Graphic State Parameters, Line Dash Pattern

@Nowass
Copy link

@Nowass Nowass commented on e65c244 Dec 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pointlessone,
thanks for your feedback ... honestly, this I didn't know that.
On the other hand, my understanding of this specification is that it zero length is not forbidden if there is another one non-zero. I would say that it depends on which segment length is zero ... for example, a segment defined by [0,2] (ON phase 0 units, OFF phase 2 units) should be still valid. Therefore the check should verify if not all values are zero, and if there are some zeroes in which phases it is.
This is my feeling which is based on how the dashed lines are interpreted by several different software (viso, inkscape, smartDraw, ...) ... but maybe I am wrong ...

P.S. as I mentioned ... if this if case is commented out, prawn-svg module knows how to handle this kind of line, and the dotted lines are rendered correctly

@pointlessone
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right about all dash length being zero. This appears to be a Prawn bug. Could you please file an issue? Also PRs are much appreciated as well. ;)

@practicingruby
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm my own understanding of the above... we're currently raising on any? values that are zero in the array, that probably should be all?

(I'll leave it up to whoever fixes this to confirm that 😄 )

@pointlessone
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@practicingruby Your understanding is correct.

@Nowass
Copy link

@Nowass Nowass commented on e65c244 Dec 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @practicingruby
I have just tried generate dotted line defined by dashed array [0,1.25] (in SVG image) with all? option and it seems to work well (PdfXchange Editor & Acrobat reader DC used for output verification)

@Nowass
Copy link

@Nowass Nowass commented on e65c244 Dec 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @practicingruby @pointlessone,
I have one stupid question ... how should I proceed to have this problem fixed? Unfortunately, I am not such programmer guru to be able to provide a "reasonable" fix :-/
From my point of view, the new issue was created (#999) ... should I take any other action ?

@pointlessone
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nowass Unfortunately we don't have much time available at the moment. So your options are either wait for us or persuade someone else to provide a fix.

I understand that is inconvenient but so is the nature of free open source work.

@Nowass
Copy link

@Nowass Nowass commented on e65c244 Dec 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pointlessone ok, clear ... do I understand correctly, that most probably it will be fixed, but nobody knows when? :-)

@pointlessone
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nowass It will definitely get fixed but yes, no one knows when.

@Nowass
Copy link

@Nowass Nowass commented on e65c244 Dec 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pointlessone OK, clear :-)

@gettalong
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nowass @pointlessone See pull request #1001 for a fix 😃

Please sign in to comment.