Hi Jens, It's really great to have your contributions. Thank you for getting involved!
If you have time, I highly recommend joining us at #sigrok IRC on FreeNode. This is where most of the sigrok crowd gathers to discuss things. With regard to your patches, most of them are good to go in, I would say. I will reply with some inline comments in a couple of cases. The big thing I wanted to discuss was your CursorHeader. I guess you put this in because the cursor markers are covering the time markings on the ruler. It's a fair point. The current design certainly isn't ideal. My concern is that having the CursorHeader won't fit very well with the other things we need to consider. In the future cursors are not going to be the time only markers that will be displayed in the ruler. There will be the trigger point indicator, and there may be other kinds of marker up there. Flags for example? If there were a trigger marker or some other markers, the CursorHeader would have to be shown all the time. So why not just integrate it in into the ruler anyway? Adding extra bars is a kind of tax on screen space, and in this case there's not much payoff for this tax. On a large screen machine this is not going to be so critical (though we still want to try and cut it back), but on mobile we really need to minimize the width of borders that we add. Many scopes get around this by drawing the markers directly onto the canvas: http://goo.gl/17pDHG , http://goo.gl/NJkou3. But I think this may be a little too intrusive on the canvas, because our markers are rather larger. I was wondering whether we could have a compomise where the markers half-cover both: http://www.airwebreathe.org.uk/space/half-cover.png The way to do this would be to take your CursorHeader and make it a transparent widget that overlays both the ruler and the canvas. This would also allow us to get around the problem we have with the selection highlight where the highlight is clipped to the edge of the ruler widget. sigrok guys: Any thoughts? Jens: Any thoughts? Joel On 22/05/14 21:03, Jens Steinhauser wrote: > These patches add small changes for stuff that I found missing when using > PulseView, and others may find useful too. > > Jens Steinhauser (13): > MainWindow: Add shortcuts for the open/save menu items. > MainWindow: Remember directory of last file that was opened/saved. > MainWindow: Add missing toolbar buttons. > MainWindow: Zoom fit after opening a capature file. > SigSession: Fix typo in comment. > SamplingBar: Show total sampling time in a tooltip. > Put the time format function into a separate file. > SamplingBar: Use nicer time format in the tooltip. > Use a separate widget to hold the cursor labels. > View: Hide the cursorheader with the cursors. > CursorHeader: Make the size dependend on the used font. > CursorHeader: Use the same number format as the ruler. > Header: Invisible traces shouldn't influence the width. > > CMakeLists.txt | 3 + > main.cpp | 2 +- > pv/mainwindow.cpp | 45 ++++++++++- > pv/sigsession.cpp | 4 +- > pv/toolbars/samplingbar.cpp | 22 ++++++ > pv/toolbars/samplingbar.h | 3 + > pv/util.cpp | 72 +++++++++++++++++ > pv/util.h | 59 ++++++++++++++ > pv/view/cursor.cpp | 6 +- > pv/view/cursorheader.cpp | 143 ++++++++++++++++++++++++++++++++++ > pv/view/cursorheader.h | 65 ++++++++++++++++ > pv/view/cursorpair.cpp | 6 +- > pv/view/header.cpp | 11 ++- > pv/view/header.h | 2 +- > pv/view/marginwidget.cpp | 4 + > pv/view/marginwidget.h | 2 +- > pv/view/ruler.cpp | 186 > +++++++++++--------------------------------- > pv/view/ruler.h | 30 +++---- > pv/view/view.cpp | 43 +++++++--- > pv/view/view.h | 2 + > 20 files changed, 518 insertions(+), 192 deletions(-) > create mode 100644 pv/util.cpp > create mode 100644 pv/util.h > create mode 100644 pv/view/cursorheader.cpp > create mode 100644 pv/view/cursorheader.h > ------------------------------------------------------------------------------ "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs _______________________________________________ sigrok-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/sigrok-devel
