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

Canvas Context setLineDash Doesn't Work #2353

Closed
wvary opened this Issue May 7, 2015 · 7 comments

Comments

Projects
None yet
5 participants
@wvary

wvary commented May 7, 2015

Hi,

I have a need to draw dashed lines in my chart.
I was using the context.setLineDash function to draw and it works fine in Chrome.

I'm getting an undefined error in wkhtmltopdf.exe.

This function is probably not supported yet.
Do you know when this feature will be supported?

In the meantime, I have to look for workarounds to implement wkhtmltopdf in my application.

FYI: I ran into the same problem with an older version of PhantomJS, but there latest PhantomJS supports it. I'm not happy with the output and performance of PhantomJS comparing to wkthmltopdf.

Thanks,

Will

@wvary

This comment has been minimized.

wvary commented Jan 28, 2016

Guys,

So really needed this setLineDash feature so I proceeded to add the code to expose that function in the canvas context. The good news is that the actual code to draw the dashed line is already written in the QT library. I just have to expose it in a few cpp files.

The question is do we want to officially support setLineDash function in wkthmltopdf. If so, should I submit my changes for review and hopefully it will be included in the next official release.

Thanks,

Will

@DeanDavis

This comment has been minimized.

DeanDavis commented Sep 15, 2016

Please submit your changes. I also need the setLineDash function, was surprised to see it not work in my outputted PDF.
It's important for printouts. The lines in a PDF may be colored but when you point them in black and white the colors may be indistinguishable. Dashed lines make them apparent in a printout.

@ygbr

This comment has been minimized.

ygbr commented Sep 15, 2016

Yes please

@wvary

This comment has been minimized.

wvary commented Sep 15, 2016

I've moved to another job and no longer have use this tool, but I like it a lot.
I'll look for the changes tonight and either will submit a PR or update this issue with the suggested code change.

I'm not a c/c++ engineer. Just know enough to add this feature.

@wvary

This comment has been minimized.

wvary commented Sep 17, 2016

I've created a PR for this fix.
The code fix is in the QT library.
You can test it before the actual PR merge:
wkhtmltopdf/qt#31

@ashkulz ashkulz closed this in 956c328 Nov 22, 2016

@ashkulz ashkulz added the Fixed label Nov 22, 2016

@ashkulz ashkulz added this to the 0.12.4 milestone Nov 22, 2016

@ashkulz

This comment has been minimized.

Member

ashkulz commented Nov 23, 2016

0.12.4 has been released, which should contain the fix for this issue. Please report back if it is not solved with the above version.

chrisbranson added a commit to Current-RMS/wkhtmltopdf that referenced this issue Dec 19, 2016

@chourie2

This comment has been minimized.

chourie2 commented Feb 24, 2017

Hello,

Since I updated wkhtmltopdf from 0.12.2.4 to 0.12.4.0 (both 32 bits, with MSVC 2013 or 2015 for the lastest) I encounter a crash when the page includes some graphics created by ChartNew.js.

This "regression" have been introduced in 0.12.4.0 (I wasn't able to reproduce this issue in any 0.12.3.x release and in 0.13.0 alpha) only. So I look into the release note and found this issue !

As it turns out, ChartNew call the function setLineDash with an empty array parameter to draw a solid line without dash. Such call leads probably to an null pointer exception on the call of context.setLineDash (line 184 of JSCanvasRenderingContext2DCustom.cpp). I'm not sure about the second paramaters of this function, is it just supposed to be the first offset for the line ? Is it necessary ?

Based on my interpretation of the documentation I would say that it's suppose to work (and it actually does in others browsers) !

Can you tell me if you think that your implementation is supposed to handle this use case ? If you don't think so, and if ChartNew doesn't use it in a proper way this function, I don't really know how should ChartNew draw non-dashed line and I would be happy to know about it to let them know about haha !

Thanks in advance for your time, I'll stay tuned if you need anything to reproduce the issue or anything ! 😄

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