1r0b1n0 / OctoPrint-Tempsgraph

Interactive temperature graph for octoprint
MIT License
18 stars 12 forks source link

I can no longer zoom Y-axe to get some more details of the temperature drift #33

Open IvanVolosyuk opened 3 years ago

IvanVolosyuk commented 3 years ago

I used to be able to go on the left side of the graph to have vertical zoom. No I cannot find this feature anymore.

Mungbeanz commented 3 years ago

Zooming in is quite broken for me too. I uninstalled the update and manually installed 0.3.6 All working as before

GhostlyCrowd commented 3 years ago

The latest update is pretty bad, this is also broken for me, it looks awful on a 4k monitor as it just expands until it pushed everything off the screen.

Roll back has it looking normal and working again.

1r0b1n0 commented 3 years ago

I guess #30 hasn't been tested on UHD devices. If someone has an idea for fix that works for all devices I'd be pleased to try it out. For the moment I'll prepare an update to revert to the previous behavior, with an optional checkbox in the configuration for the "responsive" behavior.

GhostlyCrowd commented 3 years ago

I guess #30 hasn't been tested on UHD devices. If someone has an idea for fix that works for all devices I'd be pleased to try it out. For the moment I'll prepare an update to revert to the previous behavior, with an optional checkbox in the configuration for the "responsive" behavior.

Being able to toggle the auto-scaling of the graph, and having the zoom function working properly would possibly be the best for both worlds. Perhaps some auto-scaling with some sort of logic to keep aspect ratio of the graph in mind.

Otherwise, it ends up becoming stretched vertically, and that combined with the zooming and scaling being broken it ends up looking awful and not being very useful, because it runs away vertically and becomes smashed horizontally.

IvanVolosyuk commented 3 years ago

I forgot to mention that I have vertical orientation of my monitor, may be this is relevant here, resolution is 1440x2560.

ghost commented 3 years ago

TL;DR: Screenshots, please! I'll fix this up ASAP

I just happened to notice this (sorry, didn't get a notification).

The correct configuration of plotly should get the responsive layout working regardless of resolution or aspect ratio, so no toggle should be needed in the long term, and we should be able to have fully working zoom and fully visible graphs.....but obviously more testing is needed, to make sure it will behave with different displays. I did test it on 1080p and 1440p and resized the window down to it's minimum (as per screenshots in #30 ) to be sure it works on mobile displays (eg tablets) and vertical aspect ratios, but obviously something is not right in some cases.

It will help a lot if you can take a screenshot, as "it looks bad" or "awful" or "broken" or whatever doesn't really tell me what problems I would be trying to solve. Apparently my imagination was not wild enough so far, so I don't want to leave it up to that again ;)

GhostlyCrowd commented 3 years ago

4k_Broken_Capture 4k_Broken2_Capture 4k_Capture

TL;DR: Screenshots, please! I'll fix this up ASAP

I just happened to notice this (sorry, didn't get a notification).

The correct configuration of plotly should get the responsive layout working regardless of resolution or aspect ratio, so no toggle should be needed in the long term, and we should be able to have fully working zoom and fully visible graphs.....but obviously more testing is needed, to make sure it will behave with different displays. I did test it on 1080p and 1440p and resized the window down to it's minimum (as per screenshots in #30 ) to be sure it works on mobile displays (eg tablets) and vertical aspect ratios, but obviously something is not right in some cases.

It will help a lot if you can take a screenshot, as "it looks bad" or "awful" or "broken" or whatever doesn't really tell me what problems I would be trying to solve. Apparently my imagination was not wild enough so far, so I don't want to leave it up to that again ;)

This is an example of the silliness i get on a 4k screen :) First 2 pics are latest release and example of how wild it gets and makes you scroll, last pic is previous release where it worked perfect. Also as stated before its zoomed right in and cannot be zoomed out if you look at the time stamps on the broken one vs the working one.

GhostlyCrowd commented 3 years ago

And here is an example from my mobile. Previous release works I can scroll around and do things of needed. Recent release I just get a graph on-top of everything and can't scroll around or use the interface at all. First screen shot was previous release which is usable but ill admit no one uses the browser usually an app, second screen is current release and i cant use it at all. Screenshot_20210327_173448_com.android.chrome.jpgScreenshot_20210327_173205_com.android.chrome.jpg

IvanVolosyuk commented 3 years ago

How can I get a screenshot if I just don't have the controls to zoom vertically anymore? On the older version if I click close to the left or right side of the graph I can zoom only in the vertical direction. It just doesn't happen on the newer version, so I rolled back.

On Sun, Mar 28, 2021 at 8:36 AM GhostlyCrowd @.***> wrote:

And here is an example from my mobile. Previous release works I can scroll around and do things of needed. Recent release I just get a graph on-top of everything and can't scroll around or use the interface at all.[image: Screenshot_20210327_173448_com.android.chrome.jpg] https://user-images.githubusercontent.com/1682110/112735504-e1823900-8f22-11eb-8c7c-6eb482c0d306.jpg[image: Screenshot_20210327_173205_com.android.chrome.jpg] https://user-images.githubusercontent.com/1682110/112735506-e34bfc80-8f22-11eb-868d-3e06eb108664.jpg

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/1r0b1n0/OctoPrint-Tempsgraph/issues/33#issuecomment-808805838, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABXQ6HOITJH5DRZXJOTIOCLTFZFU3ANCNFSM4ZB3V4FA .

thisiskeithb commented 3 years ago

TL;DR: Screenshots, please! I'll fix this up ASAP

The temperature graph is distorted with Themeify:

image

ghost commented 3 years ago

TL;DR seems like it's themeify putting the controls/webcam tab and graph tab on one page. How should we make this work, both for people with themeify, and for those without it (ie default installation)?

Thanks for the screenshots, that helps a lot! We can clearly see here why only some of us are having trouble while others are OK - it's not the aspect ratio/resolution - it's a conflict with themeify and having the layout where controls are beneath the graph; Because the graph expands to fill the space between the side panels, and the space has to be shared with the controls/webcam, that's why you need to scroll.

This raises the question, how would you like this to work? The replies above imply that you would rather the controls/webcam rules this display, and the graph should only be allowed to grow so large as to take up any remaining space. That's fine for the layout you have, but hat doesn't allow for other layouts, most notably the default one where the controls are on a separate tab. So... considering all users' layouts, not only those who use themeify to get this combined layout, how would you suggest this is done?

BTW, you may notice I'm not using themeify, and that is because of problems like this will other enhanced temperature graphs. There is a chance that this might need to be addressed by themeify. But anyway I will do what I can here, first.

Recent release I just get a graph on-top of everything and can't scroll around or use the interface at all

It seems like what you're saying is that in order to pan and zoom on your mobile device, you were using un-used white-space, of which there is now none (because the graph has expanded to take up the wasted space), so when you attempt to pan and zoom the page, you only succeed in panning and zooming the graph.... Am I understanding that correctly?

How can I get a screenshot if I just don't have the controls to zoom vertically anymore?

Maybe you can grab a screenshot of what you expect to be there which is there in the old version? Your more detailed description might help, I'll try zooming the way you described and see if that helps to illustrate the problem.

I'm fairly sure controlling the graph zoom is still functional, as that was one of the driving forces for this change (being able to un-zoom which we couldn't easily do before) I will double-check, and I will also add the same enhanced zooming features present on the X axis, to the Y axis, since obviously zooming that axis is also quite useful.... but even in the mobile screenshot above I can see that the zooming functionality is present. Perhaps it's just the change in process giving the impression that it's not working (in simple terms: you were zooming using one method, and now it's using a different method)

ghost commented 3 years ago

For reference in case you missed it from the PR, this is how it looks without themeify adding elements to the graph tab:

https://user-images.githubusercontent.com/58130821/102578798-1c6c1800-414f-11eb-813a-7eb5f03d0f5e.png

Probably can close #32 as we are addressing these issues together.

IvanVolosyuk commented 3 years ago

Looks like my issue was hijacked with other problems. I don't have themeify, I don't have 4K screen, though I have rotated monitor if that matters.

My issue was about not being able to zoom Y-axis only. I was able to zoom exactly from 189C to 191C to see the temperature drift deviation around 190C degree with high level of detail in the Y-axis only. I want to keep the X-axis zoom level unchanged, so I can see the highly detailed graph of temperature over a long period of time. I haven't found the new method of zooming to accomplish this goal. Can you explain how I can do that?

Zoom button is present as before and as before it doesn't allow to do what I need. My method was to start dragging on the left side of the graph near 191C mark and release mouse near 189C mark. It no longer works, and I don't see how the new method of zooming can possibly do that.

IvanVolosyuk commented 3 years ago

I think there might be a bit of misunderstanding. I'm talking about a feature which existed in version 0.3.6. As it no longer exist (works?) in the 0.3.7 I will use 0.3.6 version to explain what I'm actually missing. I could drag over left side of graph to pick zoom level for Y-axis only and keep X scale unchanged.

This was pretty much the most useful feature of the plugin. I can monitor over a long period of time how stable my noozle temperature during entire print. If I see temperature drift a lot (more than 1 degree) I can start PID autotune or check if termistor is slipped out of the right place it supposed to be (my most common failure mode). Zoom in button is useless in this respect as I want to overlook the entire duration of the print. Zoom out state lacks the detail - 1 degree drift is not very visible there.

Screenshot_20210329_121712 Screenshot_20210329_121727 Screenshot_20210329_121759

ghost commented 3 years ago

Thanks for the screenshots, I know what you mean now.

Don't worry, your issue has not been hijacked. I am the author of the patch that, for some people, improved the graphs. That same patch has unintentionally caused both your issue, and the others above. Since I have some time to spare and the patch was mine, I thought I would help out, and I will address them all together, just as the problems arrived all together.

I understand what you mean about zooming the Y axis and I thought that it was working but maybe not. I say "maybe" only because I have a broken cooling fan which obviously is critical to my testing this properly so before I can really test it I need to repair my printer. I've got parts here but I haven't had quite the time to do that one just yet, but don't worry, I am making time, I just wanted you to understand what's happening :) Anyway I'm pretty sure it was working because I was using it for a similar purpose, but I have a feeling that the way of doing it has changed which is why it's been uncomfortable for you.

Not only will i fix this so it works as before, I will also add some new features which will make zooming that axis work better and be easier to control (range slider). I did this already with the time axis but not the temperature axis, so I'll do it for both when I fix the other problems, so it will work as it did before, and also have some cool new stuff.

ruedli commented 3 years ago

Indeed this version no longer allows to zoom in for the Y-axis. I use an Ultrawide (5120x1440) monitor, use Displayfusion and Themify. The problem is that the cursor, while dragging ONLY shows X-Y borders, not a rectangle. I captured it (including the cursor).

image

Systeminfo.txt:

browser.user_agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.131 Safari/537.36 connectivity.connection_check: 1.1.1.1:53 connectivity.connection_ok: True connectivity.enabled: True connectivity.online: True connectivity.resolution_check: octoprint.org connectivity.resolution_ok: True env.hardware.cores: 4 env.hardware.freq: 1500.0 env.hardware.ram: 1902264320 env.os.bits: 32 env.os.id: linux env.os.platform: linux env.plugins.pi_support.model: Raspberry Pi 4 Model B Rev 1.4 env.plugins.pi_support.octopi_version: 0.18.0 env.plugins.pi_support.throttle_state: 0x0 env.python.pip: 20.3.3 env.python.version: 3.7.3 env.python.virtualenv: True octoprint.safe_mode: False octoprint.version: 1.7.0rc1 systeminfo.generator: zipapi

Let me know if you need specific settings / log. I was relcutant to send the generated zip file as it contained 5Mbye of (useles?) log info (like haproxy.log) I use currently the 1.7.o RC1 and all latest plugin version.

ghost commented 3 years ago

Good gracious, my sincere apologies, this fell off my to-do list somehow. I just got the notification for the above comment, I'd forgotten completely.

I have been unwell so my printer is still out of action, but the parts have arrived. I will get to work on this right away.

Themify

Thanks, this is very interesting. Your themify configuration does not add the extra elements to the temperature page, so you don't get the other issue others had above. This confirms my suspicions about why it behaved strangely for some but not others. This is good news because it means themeify isn't breaking it directly, which means compatibility between these two plugins is highly likely. (Edit: To be specific, I will have to detect whether themify has added elements, and configure plotly to save space for them if necessary - and I expect that to work)

The Y axis zooming is also very interesting, because yours is not broken by themeify, I can see that there are elements of the plot entirely missing - for example, I added a range slider and it's just not there at all. This has me scratching my head, I definitely know that was working here. I'll figure it out, don't worry.

It's normal for the zoom to lock to an axis based on your initial movement - as in, if you click and drag to the right, it will only work in the X axis (as per your image), if you click and drag downwards, it will only work in the Y axis, if you click and drag diagonally, you get the rectangle.... But I'm guessing that's not what is happening here.

Please do feel free to @ me if you don't hear from me in a reasonable time frame... and given that this went forgotten for some four months, that means any time you like. I am very sorry.

ruedli commented 3 years ago

@xcasxcursex Yes, you got the point: I am moving diagonally (and even vertically), but nomatter how I move it only extends the selection in the x-direction. I tried this in "chrome". I also tried Microsoft Edge, but it behaves exactly the same. Let me know if you want me to install a special version to try things.

ruedli commented 3 years ago

@xcasxcursex For your information: I disabled UIcustomizer. This did not make any difference other then my UI showing in the standard layout and colors. Still problem zooming in Y-direction.

ghost commented 3 years ago

Thanks mate. I have reproduced this here, and I have this entire day reserved for this job alone. I expect I should have it fixed before the day is out.

ruedli commented 3 years ago

@xcasxcursex That is excellent news, I am sure it can be fixed now you were able to reproduce it. You probably do not need more info, but here is some anyhow. It appeared during all these observations I had not the "themify" plugin active. Further, rolling back to 0.3.6 resolved the Y-axes zooming problem.

ghost commented 3 years ago

Just touching base here since I have set a poor precedent of disappearing: I have had some unexpected urgent personal stuff come up and this has been delayed by several hours - but I've not disappeared and I wont drop this til its fixed and it should not be long.

ghost commented 3 years ago

Another quick update in the interest of better communication: I have identified the exact cause of the fault and am testing a fix. Expect more news within the hour.

ghost commented 3 years ago

Fixed the Y axis zooming. I'll post a patch when it's all complete, but if you'd like to see it in action it's very simple: add fixedrange: false to the Y axis.

So, that's the bug here fixed, now I need to look into compatibility issues.

I will take my time to test and fix the layout issues across multiple browsers and display resolutions and with or without themeify running and with or without themeify modifying the temps graph page. I know you guys have been waiting for ever and I'm so sorry. I just don't think it would be wise of me to rush this part. Not only do I need to perform a bunch of tests so it's just a long queue, there is still a strong chance that I will need to patch not only this project, but also themeify (because really, the layout problems are a themeify bug, not this project's... but hopefully, I can work around them from here.)

As always, please feel free to @ me any time. I'll take a break for the day (it's been a rough one) but get right back on this tomorrow.

ruedli commented 3 years ago

@xcasxcursex Thnx for the update. It indeed fixed the y-axis zoom problem in 0.3.7. for those also want to apply this fix (I had trouble locating the file to fix):

This resolved it for me.

image

ghost commented 3 years ago

Skip to the headers at the bottom for TL;DR

Well, that layout thing turns out to be a complete PITA. I'm about to take a rest after 24 hours straight of finding new ways to break it.

First off, it turns out I've been blaming themify for the layout problems, and it wasn't themify, it was a consolidated tab plugin. Sorry, themify ;)

Specifically, this one: https://plugins.octoprint.org/plugins/consolidate_temp_control/ , in vertical mode. The author of that plugin also has a plotly based graph plugin (not quite so polished as this one) so I figured I'd use his solution, but his two plugins don't work together either (even worse than with this plugin - the temps just aren't consolidated at all). So I will have to come up with something.

The problem here is to define the height of the plot. Before, it was 400 pixels, fixed. For @GhostlyCrowd, this worked well because the display resolution and webcam resolution just so happened to make it fit nicely. Honestly, that was just luck.

At present, the plot takes up all the available space, meaning, all of the space, minus the height of toolbars and such which are normal for the temps page. This breaks it for you, Ghostly, because your modified temps page has these extra elements, which now get pushed off-screen by the graph growing to where it thinks it has space.

I could work around this by specifically detecting the effects of the other plugins, adjusting for the available space, and specially re-sizing the temps graph. Seems plausible... Until you consider that before I found the guilty party here, I came across at least 4 different 'consolidation' plugins, and you guessed it, every one of them works differently (two of them even work differently internally, depending on the way the page is laid out), making different assumptions about the size of the plot.

The crux of this is that other plugins assume that the temperature graph is the stock one. Fair enough. But the stock one is meh, so we don't want to copy it. Except, if we expect this plugin to always work wherever the stock one would, that's what we'll have to do. I could write out a special edge-case quirk for everything known but it's just not practical and who knows what will come along tomorrow and require further maintenance here.

But since Ghost was so kind as to grab me all that helpful info, I absolutely refuse to leave it as "sorry contact the other plugin author".

So this is the plan:

I've got this basically all written up but before I get it all done I wanted to check that this would be an acceptable solution for everybody. @GhostlyCrowd especially I'd like to hear from you on this because you're the most effected.

Quick zoom fix

If you want to apply the zoom fix, there is a quick way - this won't survive a reload but it is very easy.

Open a browser console (F12 on firefox, CTRL+SHIFT+J on chrome), and paste in the following:

document.getElementById("div_g").layout.yaxis.fixedrange = false; Plotly.redraw(document.getElementById("div_g"));

Press enter and the fix is applied.

ruedli commented 3 years ago

@xcasxcursex Thank you for your in depth research on which parts go wrong and where. It leaves me a little confused as to adding the fixedrange: false, to tempsgraph.js and installing that resolved the problem (apparently) for me. Are we dealing with multiple problems? from my perspective releasing this as 0.3.8 is a step in the right direction (and perhaps all that is needed?). Perhaps this should be considered as a quick fix and then check what the remaining issues are - if any?

ghost commented 3 years ago

Are we dealing with multiple problems?

Yes we are, there's the Y axis zooming thing, and a layout behaviour that was unexpected (which turns out to be specific to just one of us, but has the potential to effect us all depending on our configuration of other plugins)

There are a couple of other configuration changes that need to be made to the Y axis (eg now that the zooming works, the range slider preview isn't behaving as desired) but I will wrap all those changes up and push them today so it can be released if you want.

Then, I'll get the sizing improvements sorted (a new and more accurate and better looking graph height calculation, resizeable graph, graph size setting storage) for all the people like Ghostly who use tab consolidation and giant or tiny displays

Then I'll do #34 (dual Y axis for independent ranging/zooming/panning of different temperature sensors) because this thread makes it pretty clear how important full-featured y-axis zooming is, for you all ;)

WindArrow3d commented 11 months ago

Is this project dead? It would be great if the Y zoom worked (which is the most useful part....).

IvanVolosyuk commented 11 months ago

0.3.6 version works, so you can side-load it if you want a working version.