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

Itext with style but no fill and no stroke #2859

Closed
asturur opened this issue Mar 27, 2016 · 1 comment · Fixed by #2868
Closed

Itext with style but no fill and no stroke #2859

asturur opened this issue Mar 27, 2016 · 1 comment · Fixed by #2868

Comments

@asturur
Copy link
Member

asturur commented Mar 27, 2016

Version

1.6.0

Test Case

http://jsfiddle.net/Da7SP/39/

Steps to reproduce

Draw some iText with no fill and no stroke properties, put fill and stroke in the style instead

Expected Behavior

Text should render

Actual Behavior

Text does not render

This happen because we have a check on stroke and fill before going trought rendering.
There is a ambiguous property in Itext "_skipFillStrokeCheck" that is default to false.

Instead of setting it to true to force rendering of this kind of text, it would be better to add a proper check before rendering:

if ((!this.stroke || this.strokeWidth === 0) && this.isEmptyStyles()) {
        return;
      }
if ((!this.fill) && this.isEmptyStyles()) {
        return;
      }

and remove _skipFillStrokeCheck that is of no clear use to me.

Normal text class has no isEmptyStyles functions.

We could do in two way:

  1. check for a lazy !this.styles instead of isEmptyStyles()
  2. add a isEmptyStyles function that return true for the Text class

@kangax and @Kienz if you have 2 minutes i need your opinion on this.

@asturur
Copy link
Member Author

asturur commented Mar 27, 2016

we could do even better and add a parameter 'property' to isEmptyStyle to return true if the given property is not found in the style object.

So we can have a isEmptyStyle('font-family') isEmptyStyle('stroke') isEmptyStyle('fill')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant