jqPlot / jqPlot

A Versatile and Expandable jQuery Plotting Plugin
http://www.jqplot.com
Other
236 stars 103 forks source link

over sized charts in some browsers #119

Open bogdan-harja opened 7 years ago

bogdan-harja commented 7 years ago

Hi,

We're using jqPlot to generate our charts; we've recently upgraded to 1.0.9.

After the upgrade, on some browsers, charts appear over sized, see the attachments. normal chart oversized chart

The issue was reported to us on Safari / OS X, but we've been able to replicate it in Chrome on Win using Developer Tools -> Device Emulator -> iPad; it also appears for other devices listed there (e.g. Nexus 10).

Our analysis shows the problem is related with the jqplotToImageStr function. This because our js code does the following:

For debugging purposes we've logged the chartImg variable in the js console, right after applying the jqplotToImageStr function, than opened the resulted string as an image; the displayed image is the over sized chart.

Regards, Bogdan

lartx commented 7 years ago

Hi Bogdan,

I think I found the culprit. In jquery.jqplot.js, function initCanvas, just comment these lines added in v1.0.9 :

            var cctx = canvas.getContext('2d');

            var canvasBackingScale = 1;
            if (window.devicePixelRatio > 1 && (cctx.webkitBackingStorePixelRatio === undefined || 
                                                cctx.webkitBackingStorePixelRatio < 2)) {
                canvasBackingScale = window.devicePixelRatio;
            }
            var oldWidth = canvas.width;
            var oldHeight = canvas.height;

            canvas.width = canvasBackingScale * canvas.width;
            canvas.height = canvasBackingScale * canvas.height;
            canvas.style.width = oldWidth + 'px';
            canvas.style.height = oldHeight + 'px';
            cctx.save();

            cctx.scale(canvasBackingScale, canvasBackingScale);

No more odd scaling with jqplotToImageStr. I haven't seen any side effect yet, but check carefully nothing is broken with your application. Best regards, Lionel

johanbove commented 7 years ago

Thanks for reporting this. This should be fix with PR #122

bulldozier commented 6 years ago

I'm seeing this on pie charts as well. PR #122 only fixes funnel charts. The only workaround I have so far is to comment out the lines mentioned above by Lionel. I also have not seen any harm in commenting those lines.

bulldozier commented 6 years ago

The real fix is to use the inner element width and height when calling drawImage:

          else if (tagname == 'canvas') {
                var hh = $(el).innerHeight() - 2 * parseInt($(el).css('padding-top'), 10);
                var ww = $(el).innerWidth() - 2 * parseInt($(el).css('padding-left'), 10);

                newContext.drawImage(el, left, top, ww, hh);
            }

I verified this outputs correct sized charts on my iPhone and macbook, which draw too large without this change.

melloware commented 5 years ago

Someone should submit this as a PR.

womd commented 4 years ago

notworking just ran into this issue - wanted to let you know how to reproduce with win/linux - chromium based browsers: ( chrom version: 79.0.3945.117 ) on the testpage: http://www.jqplot.com/examples/barTest.php open devtools (select any device, or choose responsive )- reload. then click on "view Plot image" - button of "stacked bar chart" ( without dev-tools open it looks as expected ) .....

PrimeFaces also uses this library, but their showcase on "export chart" works on these systems...so it has been fixed ?!

on mac ( even with chrome or safari) you can see the problem withou opening dev-tools ( tested on MacBook Pro ( Retina 15-inch - os-version 10.15.2 - chrome version: 79.0.3945.130 )

although the difference is minor, it gets worse on bigger charts ....

GitHubUser4234 commented 2 years ago

The real fix is to use the inner element width and height when calling drawImage:

          else if (tagname == 'canvas') {
                var hh = $(el).innerHeight() - 2 * parseInt($(el).css('padding-top'), 10);
                var ww = $(el).innerWidth() - 2 * parseInt($(el).css('padding-left'), 10);

                newContext.drawImage(el, left, top, ww, hh);
            }

I verified this outputs correct sized charts on my iPhone and macbook, which draw too large without this change.

Great job @bulldozier! I also just tested this with several browsers on Android/Windows 10 using jqplot.1.0.9.d96a669 and there seems to be no more issue in any of them. This should really go into a PR.