Blendsplitter #117
Blendsplitter #117
Conversation
These are already slightly adjust to be in a flat subdirectory. Original code can be found at https://github.com/marekdedic/BlendSplitter and was MIT licensed when converted.
Simply moved the expander.png in our already existing qrc file. Mainly since I didn't feel like working out how to incorporate 2 qrc files in our buildsystem :-)
Better coloring when widget disabled. Highlight current SP address.
Currently the old view is in a splitter visible on first workspace. The old code had a lot of direct signals/slots and calls between DebuggerForm and all the *Viewer code To decouple this the SignalDispatcher was created as a central way to forward signals to all views that registers. Also the CPU registers state code now also lives in the SignalDispatcher. This means that now we can have multiple DisasmViewers since the code doesn't rely on one and exactly one disasmView to exist. As a side effect the QAction that would toggle breakpoints are now disabled, since that code can no longer as the cursorAdress from the one-and-only disasmView the old code assumed.
After splitting you end up with two SwitchingWidgets of the same type
In effect only BlendSplitter layout in use now
Also tried some fancier popup menu on the SplitterHandle, but this causes a lot of crashes. I assume that the handler is already free'ed while the mouseReleaseEvent is still being processed causing these SegFaults. Also I got divide by zero errors when creating new StackViewer with an open openMSX connection, it seems to handle the command reply before the widget is fully initialized??? Also the changing of background (when connecting/disconnecting the emulator) in the scrollarea isn't eassily triggered. If you resize by hand it will correctly change the color not covered by the single widget, but simply setting an new background doesn't cause an update, neither doesn explicitely calling update() or repaint().
This widget simply contains a richtext label to explain the new workings of the debugger. The idea is that this is the default widget for a new SwitchingWidget until the user is familiar with the workings and we have a setting to switch this behaviour to a more regular blender like splitting of the current area.
Use JSon format to save and load the current workspaces TODO: set the enabledWidget status to current connection status. Maybe save setting from the widgets in the SwitchingWidgets also?
And prevent direct emitting of the signal so that the SignalDispatcher is the single source of truth.
In preferences you can now can select a default workspace to load when starting. Also added a 'new user' setting so that the QuickGuide is clearly visible for new users. The CPU view/code debugger is now a workspace that can be added/removed.
Moved code in single class, and improved usage. ESC will now cancel any ongoing edits. Clicking on other tabs will now close the lineEdit. And the tabs are now movable.
Thanks to a quick review of Wouter
Since there is code that doesn't use ui files, we now manually set the QObject names to make debugging simpler
|
Looks good with blendsplitter! I tried this version today but it seems the codeview is not jumping to the position of PC, it is always at #0000 for me. Am I missing something here? Stepping over code F8 also does not change the code view. It also takes some time on my recent laptop to start the debugger (around 7 seconds) until the gui shows. The non-blendsplitter version is instant (for instance build 126). |
This is going to replace the PaletteDialog that is now called from the TileViewer and SpriteViewer, but was not yet used in the bitmapviewer... As soon as those viewers are adapted, we will remove the PaletteDialog
That is more because I missed something, more precisely I must have missed a signal/slot connection somewhere. Allow me to quickly explain: The old debugger had exactly one code viewer, one register viewer, one stack viewer, one main memory viewer etc. You could hide them, but there was always one instance of them in memory. And the code used that knowledge. So the code viewer held the 64 KB the cpu could see, the register viewer was responsible for updating registers and telling the code viewer that PC had changed and the stack viewer that SP had changed, the slot view told the code viewer if the visible memory had changed etc. In the blendsplitter way of working you can of zero, one or even more of every possible widget. So the new class SignalDispatcher is now kind of a data bus that is responsible for all those signals and data centralization.
I'll have to look into that. There is a lot of extra internal overhead now before the GUI can be build but going from 0 to 7 seconds seems like a lot of overhead. |
When the 'slots:' indicator is removed in the header files the names do no longer end up in the QMetaObject. This is not needed with the new Qt5 way of hooking up signal/slots, but the ui files use this to autoconnect the slots to GUI elements by the on_<widgetname>_<signalname> naming convention. This no longer worked so we are fixing that here. The VDPCommandRegViewer will be fixed more correctly later :-)
* Consistent indentation: We currently have a mix of indentation style: sometimes we use 1 tab, sometimes we use 4 spaces. I don't have a strong preference. But do use the same style within a single function (preferably also within the whole file). I guess some code was edited using an editor which did not have tab-stops at 8 spaces. This lead to horribly inconsistent (unreadable) indentation in some functions. * Various typographical conventions: ** no space before comma, space after comma ** space before and after operators ** no space before or after parenthesis
Avoid repeating the same lookup twice (and risk making a typo between the 1st / 2nd string literal).
No need to implement an empty default constructor. No need to explicitly call the default constructor of a base-class.
Let's get rid of the old PaletteDialog and use the MSXPalette as it was intended to be used.
Now that the PaletteViewer has taken over all the functions of the PaletteDialog we remove all traces of this old dialog.
The autosync is only meant for the VDP palette to sync to the current openMSX session, so make this more clear in the GUI.
Now we can decode 8 bytes as a character. And since character definitions do not need to be aligned in memory the hexviewer address can now be unaligned as well.
Keeping up to date with the latest work on the BreakpointViewer
Take decoded chars width into account. Cyan line in decoded char to indicate marker address. Allow shift+left/right when in unaligned mode to shift addresses in single byte steps.
Since openMSX 18.0-106-ge55a4a904 (commit e55a4a9044b...) the TCL command 'machine_info device VDP' now also returns the version string from the machines XML config. This is now used to select the correct view for the VDPRegViewer when decoding the VDP registers.
This is the new blendsplitter layout for the debugger.
Code is usable but still needs some extra love, feedback is welcome.
Things that still need to be implemented