-
Notifications
You must be signed in to change notification settings - Fork 100
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
QHexView 5.0 Development #71
Comments
Excellent, and thanks for notification as well! Here is what I can report so far from version downloaded yesterday:
This happened on Windows 11, on high DPI (2880x1800) display. Not sure if that matters, but the old version works fine on this system.
Porting to the new version was easy, so once that rendering issue is fixed I think I will be ready to use QHexView 5.0 full-time. |
Thanks for the feedback! |
Pubic access to So I'd say if you come up with a way to hide |
Update on the rendering issue: Tested on an older notebook (Windows 10, 1366x768 display), messed up there as well. Old version works fine on that machine. Try changing display resolution of your virtual machine, perhaps you have accidentally hardcoded some value that happens to work for you particular resolution? |
I need two informations (so I can try to reproduce this issue):
I've tried to change the resolution and used different fonts, I can't reproduce it. |
Qt version is 5.14.2. The result of this (on Windows 11):
is
Let me know if you need font results from the other Windows 10 machine. |
Looks like I found the fix for this. The issue is in Lines 206 to 219 in 34e1dcb
Convert the result of this into
Like this, for example:
This brought everything back to normal for me. I have no idea how exactly this fixes the issue, but I suppose using |
Great! Pushed! Let me know if it's ok now. |
Everything looks fine. Now, to the lesser issues:
The green color demonstrates the issue, the black one is just cursor selection, working correctly.
The
In the old version I used a small hack to adjust widget's size to what is actually being rendered (to remove that useless white space after ASCII column) by calling The whole idea of what is the best way to resize
This view has 17 rows, two of which are not seen. Scrollbar steps correctly, that is, twice, as it is supposed to. I am only pointing out its appearance. Shouldn't it be longer? For comparison, this is a scrollbar from a QTreeView with 17 root items (not expanded), it is twice as large: Other than that everything else that I use appears to work well so far. Editing works fine, both in Hex and ASCII areas, both modes, insert and overwrite work correctly. I mentioned changing font above, that works well too. |
I've fixed 1, 2, 5 and 6 along with a bunch of other minor fixes/improvements. About the other issues:
|
The reverse of that, if I understood the question correctly. But this could work too I suppose. Either way I would like widget's width (just width, not height) to be "synchronised" with the actual content, with what is being drawn (hex , ASCII areas, etc.). Right now widget draws offset, hex and ASCII columns and anything after that is wasted space (if widget's width is larger than what these 3 columns occupy). So if widget is too wide I think its width should be reduced. In the old version I did something like this:
It may have some implications since widget is programmatically resized, but the idea seems to be worth exploring.
I figured it out. New QHexView does not create QHexDocument unless it is explicitly set by Line 36 in b3c0e4a
My program crashed because of this:
The simplest way to fix is to just copy that single line from the old version. Although perhaps cursor mode could be set directly from A few more bugs:
In this screenshot there is actually one more line at offset 0xF0. It is not visible, thus scrollbar is supposed to appear, but it doesn't. Reducing widget's height slightly makes scrollbar to appear. |
I have added And I've also fixed all issues you have reported, an empty QHexDocument is created by default too, like the previous version. |
Issue 8: Now scrollbar area appears, I can see up and down arrows, but the scrollbar itself is not there, so I am still unable to scroll down to the 0xF0 line.
When Also while the widget itself correctly adjusts its width, the |
And there is also one more thing that could be restored from the old version: in Hex area bytes |
I've improved the scrollbar's behavior, issue 8 should be fixed. You can highlight any byte by using QHexOptions options;
options.bytecolors[0x00] = {Qt::lightGray, QColor()}; // {Foreground, Background}
options.bytecolors[0xFF] = {Qt::darkBlue, QColor()}; // {Foreground, Background}
hexview->setOptions(options); It's also possible to highlight bytes from the widget itself with these functions:
By default the new version doesn't highlight anything. |
It is, but one more emerged: it appears that there is some kind of invisible "scrollbar" when there is no actual scrollbar, mouse wheel can be used to trigger "it" and viewport will scroll one hex line down (so it essentially looks like the first line just disappeared). If mode is switched to insertion and you keep inserting till the new line is created it automatically does that invisible scroll too. And when you scroll with this invisible "scrollbar" it scrolls one empty line further than the last actual line.
When set in constructor it appears to be applied to the current default empty
If only the foreground is set for the byte, during painting the Line 389 in 8c688bd
Replacing it with this worked for me: if (it->background.isValid()) cf.setBackground(it->background); It takes advantage of the fact that But I have not studied the rendering code thoroughly, so without having a bigger picture I might have missed something. |
I have found an issue similar to this one, check if it's fixed now.
I have added
Fixed |
I'm afraid not. To reproduce, create an empty
I am not sure that this is a good idea to copy anything other than I think that the difference between So perhaps The only other thing that I think needs to be copied is cursor's mode. Perhaps it could be part of |
Mmmmmh, it makes sense. At this point even |
Maybe, although I can not say much here because my only interaction with Edit: I forgot to mention that I can confirm that |
Got it, fixed.
Ok, I will do this improvement (if there aren't any pending issue, so it's easier to catch regressions). |
Still acting same unfortunately.
That one with this invisible "scrollbar" should be the last bug reported here, I think all the rest are fixed now. I have got a few more secondary things to discuss, but indeed, I will postpone posting them not to rush things too much, let's deal with this last bug and that improvement we discussed earlier first. |
I can't reproduce it anymore with the case you have described. EDIT:
The readme with the sample code is updated to reflect these changes. (It's possible that I have created regressions with this change!) |
That last bug is gone, everything works like a charm! At least in the subset of The new API is more high level, doing a good job at hiding implementation details: I am now able to setup everything I need without having to access The new screenshot is very nice, gives an excellent first look impression, though it would probably look even better with Now that everything appears to be stable, the old features restored, if I am not bothering too much with suggestions, I would like to discuss a few more secondary things, I will post my thoughts a bit later. |
Good! Yes, I'm open to listen for more suggestions/fixes/improvements |
Here are a few more things:
Currently whenever I need to reset the widget with a new
This has been described in #58. Whenever I manually edit binary data I need to sent it back from QHexView to wherever it came from originally. Currently I have to filter out whether data was actually changed or whether it just were other unrelated things like metadata.
3.1 The first column, the Address one, is empty. Let's put it to some good use: let's display the number of byte that is currently being selected by cursor, e. g. 5th, 10th, 20th, etc, in decimal system. I think somewhere here you could query for the current byte at cursor position and and compose the final "Pos: xxx" string with it. And also maybe instead of Line 321 in fe26329
3.2 I have just found a solution for #59 that does not require any code changes to work, but I need a small cosmetic change. When separators are enabled, could you make it so that the last line is not drawn? Optionally, although there may be no reason to have it in the first place, since there is nothing to separate ASCII area from the right. The last vertical separator line crosses the 3.3 And about header style: |
I can take a look this weekend. Yes, I can push 5.0 to master branch after these fixes. |
And one other little thing: suppose there is a need to manually copy some byte sequence from the hex view into, let's say, some place in a C++ source code file. Currently the copied result is written like this:
This needs manual adjustment in order to be properly introduced within the C++ source code. What about introducing a way for QHexView to do this automatically (via specific
And maybe some other notations if they are popular enough. For those who need some very specific output suitable for their particular needs there should be a way to customise this process by doing something like adding some custom And finally the user needs to be given control over whether this string created via the |
Ok so:
Ok, I prefer to keep only
Mmmmh, for example? |
For example, suppose we need to copy a large byte sequence into a C++ file, let's say a sequence of 128 bytes. With curly braces notation this would take 6 characters per byte, which would result in a very long string occupying a single line of code, looking very ugly in the editor. I suggest inserting a new line character after every 16th byte (and perhaps even give an option to customise that, could be a parameter in And I suppose this feature of inserting new line characters should itself be optional (a boolean value in |
I have added
|
I checked it out, working smoothly. A few comments:
Lines 261 to 263 in 5105af5
Lines 272 to 274 in 5105af5
class QHexView : public QAbstractScrollArea
{
enum class CopyMode { Default, WithBrackets, HexArrayChar };
};
struct QHexOptions
{
struct CopyModeParameters
{
enum class BracketType { Curly, Square, Parentheses };
BracketType bracket = BracketType::Curly;
QString prepend = "0x";
QString separator = ", ";
// Or this could be even better instead:
QChar separator = ',';
bool add_space_after_separator = true;
};
};
|
In addition to my recent comment above here is an update about the bug mentioned in #71 (comment) It seems to be unrelated to scrollbars, at least to the horizontal one definitely. In the screenshot below I am unable to scroll down any further and the last line containing four |
The last commit seems to fix this issue (on linux). I see in the next days this one: #71 (comment) |
This has improved the situation but still has not fully fixed it yet though. The issue happens if high DPI mode is enabled, that is If Here is what I can tell so far: I populate widget with enough data to trigger the vertical scrollbar appearance, choose a certain height, scroll all the way down and then carefully drag the height slider (not the scrollbar) upwards and observe what happens to the last line: First, this is the intended look of it as far as I understand: I keep extending height and then some extra space appears (is this intentional?) : At some point this extra space abruptly disappears and this is where the issue occurs. Sometimes it may be just a few pixels that are hidden, sometimes noticeably more. Not sure if there is any correlation but it seemed the more I increased the widget's height the more pixels of the last line were obscured. The cycle repeats as I keep increasing the height of widget. One a side note, the issue that was fixed when |
It appears I have managed to find a solution to this issue. Although it has been awhile since I had done any math, let alone math related to GUI, so please make sure to carefully check this in case you incorporate it. The vertical scrollbar is the cause of the problem: in some cases as shown above it does not have enough range, making it impossible to scroll down to fully see the last line. The fix can be as simple as to always add another +1 to Lines 406 to 413 in 67dc02e
I have tried improving it by inserting this after the code above: int char_height = QFontMetrics(font()).height();
div_t division = std::div (viewport()->height(), char_height);
if (division.rem > (char_height / 2))
vscrollmax++; With that the last line now is never obscured when the vertical scrollbar is active. It may still happen if vertical scrollbar is not active though - when widget is too small to show all the contents but the activation of scrollbar has not kicked in yet. So probably some fine-tuning is still needed to calculate more precicely when exactly the vertical scrollbar should become active (decimal fractions instead of integers maybe, but don't take my word for it). One other minor cosmetic issue due to this is that this extra occasional white space after the last line I mentioned earlier sometimes becomes even larger. These issues are negligible though, so if everything proves to be fine in your environment I think this fix should be added. I tested it both with and without the High DPI mode enabled and it does not seem to break anything. Perhaps you could try fine-tuning it and see what comes out. |
If a +1 improves the situation it can be a rounding issue where the line's height is calculated, maybe a |
Probably, although I tried some floating point arithmetic which have not made much difference compared to the integer solution above, but I made no thorough research there. I suppose the extra challenge is the fact that viewport's height and the number of scrollbar steps are integer values. One thing I can tell for sure is that the High DPI mode is very sensitive to precision. Recently I have discovered yet another issue with it - I have noticed some misalignment of text in As it turned out, the default tab width value was an integer value of 32 and had to be recalculated with the help of I see that you are already using |
Ok wonderful branch, great job! The minimal changes to build against the Qt6 library is to remove QStringRef and .midRed(). I would suggest, if you plan to keep Qt5 compatibility, is to replace with QStringView maybe? |
@arabine Tested in a Linux system with:
Maybe, I will check if there are some integer calculations in the code |
Ok seems good to me. QStringView is available since Qt 5.10 so maybe you can use it for all the versions (5.10 is old enough I believe). I have a question: is there a way to refresh the view when the memory content has changed? My use case is an emulator, I want to view the RAM content (I can call a method to force the refreshing if needed). |
The goal is to retain compatibility with the version 5.6.3 though, according to this discussion - #69 |
Had to make some trivial changes to make the widget buildable with
Switched to this widget because it natively supports UI dark mode and overall looks much nicer than QHexEdit2 I've used before in UEFITool: LongSoft/UEFITool@cba31d8 Huge thanks for making it, and for using MIT license. Minor feature ideas (might implement some and send PRs later):
|
A minor bug to report: macOS starting with 10.7 has scrollbars hidden by default and shown only when scrolling is performed. Had to make this modification to restore the expected behavior there:
|
I see that you happen to be in possession of a high resolution monitor. There is a bug related to these. Could you confirm that it happens on your Linux and Macintosh systems as well? The bug is described here - #71 (comment). Make sure to call The reason behind it is yet to be determined, so any assistance would be appreciated. |
I have done an internal implementation (implemented as
I have found a
Fix done as you suggested
Is there some way to set a monospace font for MacOS? There is a conditional statement that may create some issues, this is the relative code: |
There is If data is changed externally I think it's sufficient to trigger an update manually with Qt's |
@Dax89, not really a fan of platform-specific hacks, and it feels like a Qt-issue more than a QHexEdit-issue, but I'll investigate those font shenanigans further. @T-640, I don't use the flag you mentioned because UEFITool is only using Qt 5 for a single legacy platform (x86 Windows XP and newer), with explicit refusal to support HiDPI on anything that is not built against Qt 6. As mentioned on https://doc.qt.io/qt-6/highdpi.html, "Qt 5 behavior assumes that AA_EnableHighDpiScaling has been set (this flag is not needed on Qt 6)". |
@Dax89, @T-640, looks like the issue is reproducible even with Qt 6.5.0 and without the explicit HiDPI flag, just resizing the window with nothing but QHexView widget slowly, a moment can be caught where the next line is already drawn (so now about a half of line is visible), but the scroll area is not yet updated (so it can't be scrolled to become fully visible). The window can then be resized back a tiny bit, making the visible potion even smaller. Resizing further back or forward fixes it, but it can be repeated indefinitely. |
Do you have an example? Because in this case, I think I have to create a document with a reference to the data (without copying it on each data update). |
I suppose that the RAM is just a buffer of bytes updated by a virtual CPU, outside the control of the widget. In this case I think QMemoryRefBuffer is what you need, because it takes an external buffer without copying it. For example: #include <QHexView/model/buffer/qmemoryrefbuffer.h>
#include <array>
static std::array<char, 8> BUFFER = {
'm', 'y', 'b', 'u', 'f', 'f', 'e', 'r'
};
// QByteArray is also accepted
QHexDocument* hexdocument = QHexDocument::fromMemory<QMemoryRefBuffer>(BUFFER.data(), BUFFER.size());
hexview->setDocument(hexdocument); If you want something even more specific you can also roll your own model inheriting from QHexBuffer class. |
The widget scrolls automatically if the cursor (or selection) is outside of visibile range, otherwise it doesn't do anything. The method |
QHexView 5.0 is now under development! The widget is almost rewritten from scratch in order to keep its codebase as simple as possible.
I'm interested to listen all feedbacks here in order to create the best release as possible (if anyone is interested, of course!).
The current widget's appearance is this one:
Changes:
Known Bugs/Missing Features:
Mouse input can be improvedSelection highlighting can be improvedHorizontal scroll is not implementedWants:
Windows support (I work in a Linux environment, but I can test it on WIndows too)Restore highlighting supportRestore comments/notes supportBranch Link: https://github.com/Dax89/QHexView/tree/5.0
Sample code
@audetto @T-640 @neomissing @aerosoul94
If you are not interested, feel free to ignore this notification/email!
The text was updated successfully, but these errors were encountered: