orbit icon indicating copy to clipboard operation
orbit copied to clipboard

Draw timers consistently in ThreadTracks

Open karupayun opened this issue 3 years ago • 2 comments

Ronald & Florian: I would like to see if you have a better idea.

Orbit has an issue when drawing timers. Sometimes empty spaces appear between them where there shouldn't be (http://screenshot/9D5HQVW7ZUHauTk). Is even more noticeable when at zooming-out a new random space appear.

This is caused by the combination of 3 things:

  • We are skipping a lot of timers going pixel by pixel.
  • We are using lines and boxes to render timers.
  • OpenGl is not extending the timers to occupt the all the pixel.

As I said, skipping pixels is not the only thing, because it can be noticeable in other tracks as well (but maybe less).

So, In this PR we are fixing the issue for ThreadTracks. This solution will be extended for TimerTracks in general but I prefer to align on the solution first because it's not the cleanest one.

We are basically extending the start_x and end_x to the border of the pixels (using floor and ceil respectively). This is enough if we only use boxes. But I tried to skip the line optimization and we are having at least 10% regression in worst cases that I don't want to pay. So, the only solution I found it is working:

  • Floor + Ceil
  • Centered the extended start_x and end_x (by adding 0.5)
  • Draw the vertical lines using the mid point between where it should be the start and the end of the box.

As a result, the rendering is faster but the code is a bit ugly, so I'm open to suggestions.

PS: I took advantage and erase an auxiliar method that wasn't very useful.

Bug: http://b/242973688. Test: Load a capture, play with zoom a bit. Result: http://screenshot/99AKTCemnYDptqg

karupayun avatar Aug 23 '22 18:08 karupayun

Making it draft while I'm still investigating. Feel free to give me input anyways.

karupayun avatar Aug 24 '22 08:08 karupayun

I made a few strict unit tests in ScopeTreeTimerData to see if we had an issue with the method, but tests are still passing, so I will set this PR ready to review as it makes sense to me.

karupayun avatar Aug 30 '22 17:08 karupayun