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

getIncrSpace errors if graph height 67px or less #146

Closed
photonstorm opened this issue Mar 10, 2020 · 7 comments
Closed

getIncrSpace errors if graph height 67px or less #146

photonstorm opened this issue Mar 10, 2020 · 7 comments
Labels
question Further information is requested

Comments

@photonstorm
Copy link

Hi,

Really interested to see where uPlot goes (after struggling to find anything like it). I noticed a strange bug in the 1.0.0 release - if you set the height to be 67, or less, you get the following error (and no graph):

Uncaught TypeError: Cannot read property 'push' of undefined
    at getIncrSpace (uPlot.esm.js:1621)
    at uPlot.esm.js:1676
    at Array.forEach (<anonymous>)
    at drawAxesGrid (uPlot.esm.js:1661)
    at paint (uPlot.esm.js:1810)
    at setScale (uPlot.esm.js:1840)
    at _setScale (uPlot.esm.js:1995)
    at autoScaleX (uPlot.esm.js:1034)
    at _init (uPlot.esm.js:2495)
    at new uPlot (uPlot.esm.js:2507)

Any height above 67 works. I mean, the graph is next to invisible, but you don't get the error. I was trying this because I really wanted to make a small in-line graph showing rendering speeds and needed it to be 64px tall.

Here is some simple test code:

        let xs = [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30];
        let vals = [-10,-9,-8,-7,-6,-5,-4,-3,-2,-1,0,1,2,3,4,5,6,7,8,9,10];

        let data = [
            xs,
            xs.map((t, i) => vals[Math.floor(Math.random() * vals.length)])
        ];
    
        const opts = {
            width: 800,
            height: 68,
            title: 'Test Height',
            scales: {
                x: {
                    time: false
                },
            },
            series: [
                {},
                {
                    label: ' ',
                    stroke: 'red',
                    fill: 'rgba(255,0,0,0.1)'
                }
            ],
        };

        new uPlot(opts, data, document.body);

I was also trying to stop the label from appearing, as I didn't need it for my series, hence trying a blank space, or null, but nothing seemed to be able to get rid of it - which meant all the label space got factored into the height, too.

Is there a way to have just the pure graph and no series labels at all?

@leeoniya
Copy link
Owner

leeoniya commented Mar 10, 2020

hey @photonstorm

uPlot will fail to find an axis/tick divisor (via incrs) if the chart dim is smaller than axis.space, you can decrease that to avoid the issue, but since you're wanting to turn the axis off altogether, just do axis.show: false.

check out https://leeoniya.github.io/uPlot/demos/sparklines.html

not to discourage you from using uPlot, but if you're just wanting a perf monitor widget or just sparklines, uPlot is massive overkill :D

have you seen https://github.com/mrdoob/stats.js?

btw, great work on Phaser & https://github.com/photonstorm/box2d-lite, waiting for part 13!

@leeoniya
Copy link
Owner

another real-time option that looks good/micro is https://github.com/danchitnis/webgl-plot

@photonstorm
Copy link
Author

Thanks for the quick response. I've used Stats.js a lot in the past, but as you know, it has a lot of limitations - after all, it was only really designed to do one thing, and it does that pretty well, but if you need to push it any further you're out of luck (there's no history, you can't resize the graph without losing "data", it's hard to add gridlines, etc).

After messing around with Charts.js I gave up because it has no ESM support and while dispairing at that, someone in their issues list mentioned uPlot and I love it :) Tiny, fast, canvas, perfect.

I need a lot of the features it provides, so it's a good choice for me. This isn't for in-game stats or anything, it's for visualizing a huge load of Phaser 4 metrics, such as cached draws, fps, max texture use, sprite logic time, etc.

I've managed to turn the axis off, which is great :) But for some reason, it still generates the under div, above the canvas, which is pushing the canvas 64px down. If I hard-code remove the height property from the under div, then the graph appears in the correct place. There's still the over div below it, but it's at least hidden, although it does still contain a few things (the select class, which has zero width / height, and the cursor-pt which has css set on it). I've disabled the cursor in my config btw.

Honestly, I really like using uPlot so far and will be happy to contribute towards docs and stuff once I understand what's going on better.

@photonstorm
Copy link
Author

Ahh, it needs the uPlot.css file! Inject that into the dom and all is well and no hard-coded positions required. Still generates the over/under/cursor divs, but they're properly positioned now so all is good.

Going to close this off, as the issue is resolved and I know what it feels like having lingering open issues on a repo :)

@leeoniya
Copy link
Owner

leeoniya commented Mar 10, 2020

But for some reason, it still generates the under div, above the canvas, which is pushing the canvas 64px down.

sounds like you're missing /dist/uPlot.min.css?

EDIT: you beat me to it

Honestly, I really like using uPlot so far and will be happy to contribute towards docs and stuff once I understand what's going on better.

happy to hear that :)

#107 might interest you as well. i plan to use the r-tree-based https://github.com/mourner/flatbush for it at some point.

@leeoniya leeoniya added the question Further information is requested label Mar 10, 2020
@leeoniya
Copy link
Owner

Still generates the over/under/cursor divs

this looks like an oversight.

.cursor-pt divs should definitely not exist when cursor.show: false. i've opened #148 for this.

.select, is a bit trickier, since the .setSelect() API does not require a cursor, and should able to function without it. i will add select.show: false to the opts so it can be disabled.

the .over and .under divs are expected since i don't really know what external plugins might put in them. i'm not as convinced that expanding the API surface to allow for disabling them is a good idea.

@leeoniya
Copy link
Owner

these fixes are pushed to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants