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

Small changes of measurement tool #3299

Conversation

Nirus2000
Copy link
Member

@Nirus2000 Nirus2000 commented Apr 19, 2023

  • Changing the measurement tool icon
  • Add separator in the toolbar

With the addition of separators, the tools are clearly divided into:
Tools | TimeSeries | Settings

Furthermore, the crosshairs can then also be added to the tools.

- Changing the measurement tool icon
- Add separator in the toolbar

With the addition of separators, the tools are clearly divided into:
Tools | TimeSeries | Settings

Furthermore, the crosshairs can then also be added to the tools.
@Nirus2000
Copy link
Member Author

@OnkelDok

I will use this PR to mention the following problems with the measuring tool.

1.) The labels are overlaid.
grafik

2.) When displaying with marker lines, the label is also overlaid
grafik

3,) Draw speed
I have an 8 Core CPU with 32GB (AMD Ryzen™ 7 5800X, processor).
The more that has to be drawn, e.g. values etc., the more speed is lost when drawing.
I have the fear that there will be increased excitement and grumbling here if the measurement tool does not work very smoothly.

drawspeed.mp4

@OnkelDok
Copy link
Member

OnkelDok commented Apr 19, 2023

@Nirus2000 My first guess why measure tool is drawn in background is because the measure tool is not the last registered customPaintListener.
I will try something around to handle this.

Ok, the performance drop is bad. I don't know if we can keep the current drawing with customPaintListener. Or if we need to draw the measurement tool in a different way. Because it seems the customPaintListener is not designed for "realtime" drawings.

EDIT: The disadvantage of the current situation is that when the measure tool needs a redraw, then a redraw of the complete chart is triggered. But actually only the measuring lines would have to be deleted and redrawn.

@Nirus2000
Copy link
Member Author

@OnkelDok
I've installed your "Crosshair" for fun and tried... if that is still active, then nothing works. 👎🏻

@OnkelDok
Copy link
Member

I've installed your "Crosshair" for fun and tried

What do you mean with "your" Crosshair? Don't know that I provided a crosshair which can be installed. But I assume I misunderstand your comment. Can you please explain with more detail?

@Nirus2000
Copy link
Member Author

What do you mean with "your" Crosshair? Don't know that I provided a crosshair which can be installed. But I assume I misunderstand your comment. Can you please explain with more detail?

#3004 (comment) this one...

@OnkelDok
Copy link
Member

OnkelDok commented Apr 19, 2023

#3004 (comment) this one...

Ah ok, I understand. And what is not working? Do you have a screenshot or video? Maybe it's due to some logical changes/improvements @buchen has made.

EDIT: I've proposed a fix for the wrong drawing order.

Regarding the perfomance, maybe we have to draw on a separate canvas/area and not on the chart plot area. I believe the tool tip is also drawn as separate element. When I have time the next days, I'll take a look at it. Maybe I'll come up with something.
If anyone has any solutions or ideas, keep them coming.

@buchen
Copy link
Member

buchen commented Apr 22, 2023

Regarding the perfomance, maybe we have to draw on a separate canvas/area and not on the chart plot area. I believe the tool tip is also drawn as separate element. When I have time the next days, I'll take a look at it. Maybe I'll come up with something.

What I can think of: the chart is drawn into a an image. And then the drawing of the measurement is taking the image, drawing the measurement, and drawing it on the plot area. But of course this is a substantial change - possible also to SWTChart.

I believe ´the tooltip is a separate UI element of the operating system - a different "window". Then the OS takes care.

@buchen
Copy link
Member

buchen commented Apr 22, 2023

BTW, just to mention on my Mac (Apple M1, 32GB) the painting is super smooth.

Bildschirmaufnahme.2023-04-22.um.09.43.26.mp4

@buchen buchen merged commit db4b4ab into portfolio-performance:master Apr 22, 2023
2 checks passed
@buchen
Copy link
Member

buchen commented Apr 22, 2023

Okay, I merged this PR.

Unfortunately, this closes the PR and hence "closes" the performance discussion. Shall we move it to a issue ticket (I assume it is not easy to solve)

@OnkelDok
Copy link
Member

BTW, just to mention on my Mac (Apple M1, 32GB) the painting is super smooth.
Bildschirmaufnahme.2023-04-22.um.09.43.26.mp4

@buchen can you please test again with a bigger time interval? Start date around 2005, like in video from @Nirus2000.

Unfortunately, this closes the PR and hence "closes" the performance discussion. Shall we move it to a issue ticket (I assume it is not easy to solve)

Yes, a separate ticket would be good.

@Nirus2000 Nirus2000 deleted the Improvement-of-the-new-measuring-tool branch May 8, 2023 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants