Git Product home page Git Product logo

Comments (94)

T-640 avatar T-640 commented on July 17, 2024 1

Excellent, and thanks for notification as well!

Here is what I can report so far from version downloaded yesterday:

  1. Rendering is messed up, take a look:

HexMessedUp 2880x1800

This happened on Windows 11, on high DPI (2880x1800) display. Not sure if that matters, but the old version works fine on this system.

  1. If QWidget::show is called and QHexDocument is not set for the widget a crash occurs.

  2. qhexutils.h and qhexutils.cpp are not included in the .pri file.

  3. The lines separating columns are gone, could you bring them back please? They could be another optional feature. And there also could be an option to style them like it is done for QFrame's VLine shape (plain, raised, sunken styles), take a look at screenshot in QFrame's documentation - https://doc.qt.io/qt-5/qframe.html

  4. Highlighting is restored and appears to be functional, so you might want to remove it from the "to do" list above. Also I see that it has a higher level API now, with functions like QHexMetadata::setBackground it is much more convenient to use, well done!

  5. The API has become more terse, which is good of course. The only renamings I am not sure about are from QHexView::hexCursor and QHexView::hexDocument functions, I believe just the "cursor" and "document" should be enough because:

  • Anyone can see in the header file that they return QHexCursor and QHexDocument, so adding an extra "hex" in function names seems to be redundant;
  • It is obvious from the context what kind of cursor and document is being dealt with here. What else could that be? Are you concerned about confusion with similar functions of QTextEdit? I don't think that there is a problem here.

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.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024 1

Looks like I found the fix for this.

The issue is in QHexView::renderDocument, in this block:

QHexView/qhexview.cpp

Lines 206 to 219 in 34e1dcb

// Hex Part
for(auto column = 0u; column < this->options()->linelength; )
{
QTextCharFormat cf;
for(auto byteidx = 0u; byteidx < this->options()->grouplength; byteidx++, column++)
{
QString s = linebytes.isEmpty() || column >= static_cast<qint64>(linebytes.size()) ? " " : QHexUtils::toHex(linebytes.mid(column, 1)).toUpper();
quint8 b = static_cast<int>(column) < linebytes.size() ? linebytes.at(column) : 0x00;
cf = this->drawFormat(c, b, s, Area::Hex, line, column, static_cast<int>(column) < linebytes.size());
}
c.insertText(" ", cf);
}

Convert the result of this intoQString:

QHexUtils::toHex(linebytes.mid(column, 1)).toUpper();

Like this, for example:

QString(QHexUtils::toHex(linebytes.mid(column, 1)).toUpper())

This brought everything back to normal for me. I have no idea how exactly this fixes the issue, but I suppose using QStrings instead of QByteArrays when drawing could be more reasonable choice. So perhaps you should make sure that QByteArrays get converted to QStrings within functions that deal with rendering.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024 1

I have found an issue similar to this one, check if it's fixed now.

I'm afraid not. To reproduce, create an empty QHexView, and use mouse wheel to scroll down. Despite having no data and without a visible scrollbar it will still scroll down to offset 0x10.

I have added QHexView::resetDocument which copies QHexMetadata...

I am not sure that this is a good idea to copy anything other than QHexOptions. Suppose we are done dealing with a particular QByteArray and load a new one, creating a new document and calling QHexView::resetDocument. The new QByteArray may require completely different highlighting, comments, etc., but the old metadata being copied will possibly mess everything up. And I suppose same applies to base address being copied too, although I have never used it.

I think that the difference between QHexOptions and QHexMetadata is that the former is something that it is more generic, and thus stable and should be preserved, while the latter is more volatile because of being strongly coupled with particular binary data supplied to the widget.

So perhaps QHexOptions should be applied to the whole QHexView instead of just the underlying QHexDocument? Pretty much like QPalette: We've got QWidget::palette to get a copy of it, change whatever we need in it, then set it back via the QWidget::setPalette. Same could be be done with QHexView: you could add QHexView::hexOptions and QHexView::setHexOptions to operate in a similar fashion. QPalette and QHexOptions are even similar due to the fact that they are both "simple" copiable classes, while QHexMetadata is a QObject subclass.

The only other thing that I think needs to be copied is cursor's mode. Perhaps it could be part of QHexOptions too then?

from qhexview.

T-640 avatar T-640 commented on July 17, 2024 1

Maybe, although I can not say much here because my only interaction with QHexCursor was switching its mode. Though what is definitely needed is a clearer idea of what is supposed to belong where: what to QHexView itself and what to QHexDocument. The way I see it - something general affecting the whole widget's appearance and workflow - it is QHexView's domain, anything else that deals with particular binary data set is up to QHexDocument to work with.

Edit: I forgot to mention that I can confirm that QHexView::setAutoWidth is working correctly now.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024 1

That last bug is gone, everything works like a charm! At least in the subset of QHexView features that I am actively using no regressions were spotted.

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 QHexMetadata, QHexCursor pointers, and my subclass of QHexView now uses less code and is more readable.

The new screenshot is very nice, gives an excellent first look impression, though it would probably look even better with setAutoWidth(true) called.

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.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024 1

I have managed to find a quite obscure bug, which might be related to Windows only.

For an unrelated task I had to call setlocale (LC_ALL, ".ACP") (which fixes the FormatMessageA producing an incorrect output in non-English locales in case you are wondering).

This in turn messed up the ASCII part of hex view, in particular when it came to drawing characters for byte codes which do not represent any English letters. Some of them were drawn noticeably larger then the rest of characters in the ASCII area, which resulted in uneven height for entire rows.

Moreover, Qt reported this error when data was loaded into hex view:

DirectWrite: CreateFontFaceFromHDC() failed (Indicates an error in an input file such as a font file.) for QFontDef(Family="8514oem", pointsize=9.75, pixelsize=20, styleHint=2, weight=50, stretch=100, hintingPreference=0) LOGFONT("8514oem", lfWidth=0, lfHeight=-20) dpi=192

What seemed to fix the issue was replacing the std::isprint with QChar::isPrint here:

QHexView/qhexview.cpp

Lines 589 to 595 in 0cb2644

// Ascii Part
for(auto column = 0u; column < m_options.linelength; column++)
{
auto s = linebytes.isEmpty() ||
column >= static_cast<qint64>(linebytes.size()) ? QChar(' ') :
(std::isprint(static_cast<quint8>(linebytes.at(column))) ? QChar(linebytes.at(column)) : m_options.unprintablechar);

With that applied I observed the dot character being drawn instead of that odd oversized one. I have no idea whether I am doing it right but my gut feeling tells me it is better to use Qt when it comes to dealing with locales instead of the standard library.

QHexView, apart from some bugs, appears to be almost complete. The major general issue that is left to deal with is this #71 (comment), and the dialogs for performing search need some fixes here and there, and then there are a few minor things mentioned in a couple of previous posts.

Considering that, perhaps it is time to make one final push and finally supersede the 4.0 version? If you have any free time available, of course. I would be glad to assist with testing.

And probably one last thing I would like to ask for is a convenient way to find out whether the changes made to the data were fully reversed or not when the CTRL + Z combination was used. Not sure what is the right way to do that but the first thing that comes to my mind is a change in QHexView::dataChanged signal's third parameter: it could become a flags parameter, holding both information about the reason of change and whether it was fully reversed or not.

Also earlier we were talking about the possibility of introducibgQHexView::fullDataChanged signal as an optional alternative to the QHexView::dataChanged one, but now I am not that sure if it would be that useful, at least I myself in my current project find the granular approach of QHexView::dataChanged superior. Although perhaps someone else could find it useful, so if it does not mess up the design it could still be optionally included.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024 1

@arabine
I have added some shims in order to support both Qt5 and 6 with minimal changes, CMake is also updated: now it looks for Qt6 first, otherwise it fallbacks to Qt5.

Tested in a Linux system with:

  • Qt 6.4.3
  • Qt 5.15.8

@T-640

I see that you are already using QFontMetricsF and qreal a lot, but perhaps there is still some place where integers are used instead, which is causing the issue?

Maybe, I will check if there are some integer calculations in the code

from qhexview.

NikolajSchlej avatar NikolajSchlej commented on July 17, 2024 1

Had to make some trivial changes to make the widget buildable with vc141_xp target in Qt 5.6.3:

  • replacing std::isxdigit with isxdigit from <cctype>
  • replacing QString.first() and QString.last() calls with at(0) and at(1), as that string is 2 characters long.

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):

  • STATIC_READ_ONLY option that ifdefs all editing-related code away
  • similar static options to disable unused features, i.e. large file support, editable memory buffers, commands, etc.

from qhexview.

NikolajSchlej avatar NikolajSchlej commented on July 17, 2024 1

Dark mode in Linux with Qt 6.2.4, global scaling 200% on 5K display. Works really well!
Screenshot_20230423_031150

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024 1

@NikolajSchlej

replacing std::isxdigit with isxdigit from

I have done an internal implementation (implemented as QHexUtils::isHex()) so we'll avoid compiler shenanigans entirely!

replacing QString.first() and QString.last() calls with at(0) and at(1), as that string is 2 characters long.

I have found a front() and back(): both replaced replaced with at() .

A minor bug to report: macOS starting with 10.7 has scrollbars hidden by default and shown only when scrolling is performed.

Fix done as you suggested

Also, the default font in macOS is somehow not a monospace one, so I had to manually call setFont() to set it back to sanity

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:
https://github.com/Dax89/QHexView/blob/5.0/qhexview.cpp#L36-L40

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024 1

The widget scrolls automatically if the cursor (or selection) is outside of visibile range, otherwise it doesn't do anything.

The method scroll() is inherited from QAbstractScrollView and shouldn't be used.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024
  1. I will investigate this issue: it doesn't happen on my Windows 10 VM
  2. Fixed
  3. Added qhexutils.h and qhexutils.cpp files in .pri file
  4. Restored as separator option (in QHexOptions), it's also possible to change the color: currently is drawn as a simple QPainter's line.
    It's also possible to use a QFrame's VLine as you asked, but we lose the ability to change its color, even the styles (Plain, Raised and Sunken) doesn't seems to be honored for VLine/HLine
  5. Done
  6. I can change that, but a cursor method already exists because QWidget implements that and QHexView hides that one.
    A possible solution is to remove hexCursor from QHexView and rename hexDocument as document,

Thanks for the feedback!

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

A possible solution is to remove hexCursor from QHexView

Pubic access to QHexCursor from QHexView is needed to at least being able to switch modes. Removing would make sense if you can completely hide QHexCursor within QHexView's implementation, and add functions like QHexView::setCursorMode to relay calls to the underlying cursor. But QHexCursor has more than a dozen public functions, so adding all these 'proxy' functions would likely clutter QHexView's own API.

So I'd say if you come up with a way to hide QHexCursor in QHexView go for it, if not let the "hexXXX" names stay for consistency then.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

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?

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

I need two informations (so I can try to reproduce this issue):

  • The font selected by the widget
  • It might be useful to know the Qt version

I've tried to change the resolution and used different fonts, I can't reproduce it.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Qt version is 5.14.2.

The result of this (on Windows 11):

qDebug () << hex_view.font() << QFontDatabase::systemFont(QFontDatabase::FixedFont);

is

QFont(Courier New,9,-1,2,50,0,0,0,0,0) QFont(Courier New,9,-1,2,50,0,0,0,0,0)

Let me know if you need font results from the other Windows 10 machine.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Great!
It must be a nasty implicit cast with some Qt/Compiler/OS specific combo

Pushed! Let me know if it's ok now.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Everything looks fine. Now, to the lesser issues:

  1. In Hex area background highlighting grabs an extra character (an empty character that separates representation of bytes). In ASCII area it works fine, highlighting from cursor selection is correct too.

Extra space

The green color demonstrates the issue, the black one is just cursor selection, working correctly.

  1. When copying bytes from hex area via Ctrl + C the last byte is never copied.

Last hex copy

The FF byte will not be copied.

  1. Regarding the #57

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 QHexRenderer::documentWidth, but it is gone now. I was giving user an option to change the size of QHexView by making its font larger, and then readjusted its width via a call to that function. But the best solution would probably be to implement some function like QHexView::adjustWidthToContent(bool) or make it default behaviour of QHexView altogether. I suppose the situation when widget becomes too large needs to be considered in that case, or when user wants to set fixed width for it.

The whole idea of what is the best way to resize QHexView may need more speculation though, because resizing such widget correctly seems to be more challenging than doing it with others. Changing its font and adjusting width accordingly is what I could come up with, it is a bit stiff but fairly simple. Maybe there is a better idea, but for now this will do.

  1. It still crashes unless I set QHexDocument within my subclass of QHexView first.

  2. Alternate row colour starts in the wrong place:

Rows

  1. Vertical scrollbar. Probably not a bug, but rather an aesthetical preference. It feels like it should be larger. Take a look:

Small scrollbar

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:

Large scrollbar

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.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

I've fixed 1, 2, 5 and 6 along with a bunch of other minor fixes/improvements.

About the other issues:

  • 3: Do you want that the widget calculates the line's length according to its width?
  • 4: I can't reproduce it, is it possible to have a sample code which causes this issue?

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Do you want that the widget calculates the line's length according to its width?

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:

hex_view->setMaximumWidth(m_renderer->documentWidth());.

It may have some implications since widget is programmatically resized, but the idea seems to be worth exploring.

4: I can't reproduce it, is it possible to have a sample code which causes this issue?

I figured it out. New QHexView does not create QHexDocument unless it is explicitly set by QHexView::setDocument. In the old version, an empty one was always created at the end of the constructor:

this->setDocument(QHexDocument::fromMemory<QMemoryBuffer>(QByteArray(), this));

My program crashed because of this:

  1. QHexView instance is created, QHexDocument is not set (it will be when something is actually loaded);
  2. QSettings instance is being read to retrieve "insert" or "overwrite" mode setting and calls hex_view->hexDocument()->cursor()->setMode();;
  3. But because QHexDocument has not been set yet, QHexView::hexDocument returns nullptr and thus crash occurs.

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 QHexView, with something like QHexView::setCursorMode, thus bypassing the need to call QHexView::hexDocument? Unless it is designed to handle multiple cursors, like QTextEdit though. And it could unnecessarily bloat QHexView's public API too I suppose, so I am not sure.

A few more bugs:

  1. Alternate colour is still drawn incorrectly, but in a different way:

Alternate still incorrecy

  1. At certain widget height scrollbar does not appear when it is supposed to:

F0 hidden

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.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

I have added QHexView::setCursorMode and QHexView::setAutoWidth in order to rescale the viewport's width according to the rendered area, let me know if it's ok now.

And I've also fixed all issues you have reported, an empty QHexDocument is created by default too, like the previous version.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

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.

QHexView::setAutoWidth:

When QByteArray supplied is large, that is, when vertical scrollbar needs to appear, it does readjust width successfully. However, there are two occasions when it does not do that: when QHexDocument is empty and when the size of QByteArray is small (when there is no need for scrollbar).

Also while the widget itself correctly adjusts its width, the QAbstractScrollArea::viewport does not seem to - horizontal scrollbar appeared and I could scroll a lot to the right.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

And there is also one more thing that could be restored from the old version: in Hex area bytes 00 and FF were coloured differently from the rest, and dots in ASCII area were too. Now that there is QHexOptions it would make a good optional feature. Maybe even provide an option to change colour of these sort of special bytes.

Different color

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

I've improved the scrollbar's behavior, issue 8 should be fixed.

You can highlight any byte by using QHexOptions::bytecolors map, for example:

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:

  • void QHexView::setByteColor(quint8 b, QHexColor c)
  • void QHexView::setByteForeground(quint8 b, QColor c)
  • void QHexView::setByteBackground(quint8 b, QColor c)

By default the new version doesn't highlight anything.

Screenshot
Screenshot_20220305_105006

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

issue 8 should be fixed

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.

void QHexView::setByteColor(quint8 b, QHexColor c)

When set in constructor it appears to be applied to the current default empty QHexDocument. But when it is replaced via the QHexDocument::setDocument, the new one does not receive these options. I guess there should be some optional way to preserve previous QHexOptions.

void QHexView::setByteForeground(quint8 b, QColor c)

If only the foreground is set for the byte, during painting the QHexOptions::linealternatebackground is ignored and QPalette::Base is applied instead:

cf.setBackground(it->background.isValid() ? it->background : this->palette().color(QPalette::Base));

Replacing it with this worked for me:

if (it->background.isValid()) cf.setBackground(it->background);

It takes advantage of the fact that QHexView::drawFormat is being called before the row is coloured with an alternate colour.

But I have not studied the rendering code thoroughly, so without having a bigger picture I might have missed something.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

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.

I have found an issue similar to this one, check if it's fixed now.

When set in constructor it appears to be applied to the current default empty QHexDocument. But when it is replaced via the QHexDocument::setDocument, the new one does not receive these options. I guess there should be some optional way to preserve previous QHexOptions.

I have added QHexView::resetDocument which copies QHexMetadata, QHexOptions and its base address to the new QHexDocument.

If only the foreground is set for the byte, during painting the QHexOptions::linealternatebackground is ignored and QPalette::Base is applied instead:

cf.setBackground(it->background.isValid() ? it->background : this->palette().color(QPalette::Base));

Replacing it with this worked for me:

if (it->background.isValid()) cf.setBackground(it->background);

Fixed

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Mmmmmh, it makes sense.

At this point even QHexCursor should be part of QHexView: so it's possible to have multiple views with the same QHexDocument but different QHexCursor and QHexOptions.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

I'm afraid not. To reproduce, create an empty QHexView, and use mouse wheel to scroll down. Despite having no data and without a visible scrollbar it will still scroll down to offset 0x10.

Got it, fixed.

Maybe, although I can not say much here because my only interaction with QHexCursor was switching its mode. Though what is definitely needed is a clearer idea of what is supposed to belong where: what to QHexView itself and what to QHexDocument. The way I see it - something general affecting the whole widget's appearance and workflow - it is QHexView's domain, anything else that deals with particular binary data set is up to QHexDocument to work with.

Ok, I will do this improvement (if there aren't any pending issue, so it's easier to catch regressions).

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Got it, fixed.

Still acting same unfortunately.

Ok, I will do this improvement (if there aren't any pending issue...

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.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Still acting same unfortunately.

I can't reproduce it anymore with the case you have described.
Btw, I have pushed another commit which disables the vertical scroll entirely if the scrollbar is not visible, check now!

EDIT:
I've reorganized API and class relations in this way:

  • QHexView: Owns QHexCursor, QHexMetadata and QHexOptions
  • QHexCursor: Keep che cursor position and insert/overwrite state, it's always owned by a single QHexView
  • QHexDocument: Has the internal buffer, editing and undo/redo information (now it can be attached to multiple QHexView)

The readme with the sample code is updated to reflect these changes.

(It's possible that I have created regressions with this change!)

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Good!

Yes, I'm open to listen for more suggestions/fixes/improvements

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Here are a few more things:

  1. Resetting data of QHexDocument

Currently whenever I need to reset the widget with a new QByteArray I create a fresh QHexDocument with the new data and call QHexView::setDocument. Is this the right approach, or perhaps it is an overkill to create a whole new document just to reset data? Would having a slot QHexView::setData(QByteArray) be a better idea? In this case some other things might need to be taken care of though, like resetting metadata.

  1. Notifying when data is manually edited

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.

  1. Header

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.

Position and override

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 rightJustified there should be an option to justify at centre or to the left (and for the ASCII label too).

QString addressheader = m_options.addresslabel.rightJustified(this->addressWidth()), hexheader;

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.

Line crosses widget

The last vertical separator line crosses the QComboBox, so I would like it to be removed.

3.3 And about header style: QHexView in its appearance has some similarity with QTreeWidget and QTableWidget: they all have a header. Perhaps QHexView's header could borrow the default style of headers of these widgets (optionally, of course, there is nothing wrong with the current style). I am only asking because I have a QTreeWidget next to the QHexView in my program, and them having same header style would make appearance more consistent.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Resetting data of QHexDocument

I have added as QHexView::setData and QHexDocument::setData, when this method is called metadata and undo/redo stack is cleared (this happens even with QHexView::setDocument too).

Notifying when data is manually edited

I'm thinking to add dataChanged(const QByteArray&, ChangeReason) signal, the enum ChangeReason is needed so it's possible to know if it was an Insert, Replace or Remove change.

Header & more...

I replaced QHexOptions::separator with a more generic QHexOptions::flags which accepts a bitfields enum: https://github.com/Dax89/QHexView/blob/5.0/model/qhexoptions.h#L12, so it's possible to add more options without filling the struct with a lot of booleans.

About the header I have added QHexFlags::StyledHeader, QHexFlags::StyledAddress and the combined version QHexFlags::Styled which draws the background as QPalette::Window.

Separator's lines are now simpler: they are just drawn vertically and I have removed the last column (the one beyond ascii).

It's also possible to customize the address header column with QHexView::setDelegate (this allows to custom highlight data without relying to QHexMetadata too):

// customdelegate.h
#include <QObject>
#include "QHexView/model/qhexdelegate.h"

class CustomDelegate : public QHexDelegate
{
    Q_OBJECT

    public:
        explicit CustomDelegate(QObject *parent = nullptr);
        QString addressHeader(const QHexView* hexview) const override;
};

//customdelegate.cpp
#include "customdelegate.h"

CustomDelegate::CustomDelegate(QObject *parent) : QHexDelegate{parent} { }

QString CustomDelegate::addressHeader(const QHexView* hexview) const
{
    return QString("Pos: %1").arg(hexview->hexCursor()->selectionLength() / hexview->options().linelength, 0, 16);
}

//mainwindow.cpp
#include "customdelegate.h"
...
hexview->setDelegate(new CustomDelegate(hexview)); // Doesn't takes the ownership automatically
...

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

I have added as QHexView::setData

Tested, working well.

I'm thinking to add dataChanged(const QByteArray&, ChangeReason) signal

Good idea, not sure if I will find any practical usage for the ChangeReason enum, but the more well-defined public API there is the merrier.

Header & more...

Delegate works, but the CustomDelegate::addressHeader should be this instead

QString CustomDelegate::addressHeader (const QHexView* hexview) const override
{
    HexPosition position = hexview->hexCursor()->position();
    return QString("Pos: %1").arg(position.line * 16 + position.column);
}

And while we're at it, add CustomDelegate::asciiHeader too? I am not sure if I'll personally ever use it though, but it may be handy to have it just in case.

The horizontal line that separates header from the content is gone, could you bring it back please? It could be made optional just like the rest of separators.

QHexFlags::StyledAddress does not do anything, it is not referenced anywhere other than qhexoptions.h (at least in the version I downloaded, I see that you keep working while I am writing this).

I have used QHexFlags::Styled, but it gives the wrong colour (the one on the left is QTreeWidget):

Wrong color

But header style is more than just a colour. I remember on Windows XP and Windows 7 there were nice gradients (I'll try to compile on one of these systems and show a screenshot). Although now it may be irrelevant, at least in Windows 10 and 11 UI elements are becoming flat, plain and ugly, so I guess things like gradients may be banished now. But if it won't be a Herculean effort to implement this, making sure that the header style of QHexView is synchronised with whatever header style Qt has chosen to draw should be a good idea, with an option to customise, of course.

Also, two issues can be closed:

#69 - I have confirmed it works now
#55 - I think this was fixed a long time ago, even in the old version. I am calling setFont and everything is fine.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Compiled on Windows 7, That's how Qt draws header here:

Windows 7 header

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Good idea, not sure if I will find any practical usage for the ChangeReason enum, but the more well-defined public API there is the merrier.

Implemented (I'm thinking to add the offset information too)

And while we're at it, add CustomDelegate::asciiHeader too? I am not sure if I'll personally ever use it though, but it may be handy to have it just in case.

Implemented

The horizontal line that separates header from the content is gone, could you bring it back please? It could be made optional just like the rest of separators.

Restored

But header style is more than just a colour. I remember on Windows XP and Windows 7 there were nice gradients (I'll try to compile on one of these systems and show a screenshot). Although now it may be irrelevant, at least in Windows 10 and 11 UI elements are becoming flat, plain and ugly, so I guess things like gradients may be banished now. But if it won't be a Herculean effort to implement this, making sure that the header style of QHexView is synchronised with whatever header style Qt has chosen to draw should be a good idea, with an option to customise, of course.

It's possible (https://doc.qt.io/qt-5/qstyle.html#drawControl) but it's tricky because the widget is rendererd through QTextDocument (header included), afaik I need to change the rendering code a lot in order to implement that.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Working fine, just a few minor appearance issues:

  1. QHexFlags::StyledAddress grabs some extra space after separator (see screenshots of my last posts above);
  2. QHexFlags::StyledHeader on the other hand, stops a few pixels before the end of widget (that could be seen there as well).
  3. When separators are enabled, the metadata highlighting misses a few pixels from the beginning and the end of line, alternate row colour misses some pixels at the beginning and at the end of the widget (might be hard to notice, you may need to zoom in):

Color issues

But this may probably be the issue with separators themselves, drawn in the middle of empty characters instead of before or after them? I like separators drawn that way though, with some extra space to the left and right from them. So perhaps you could find a way for row colours, selection and metadata highlighting to take separators into account when being painted?

I'm thinking to add the offset information too

Yea, why not, if it does not complicate the code. No reliable way of knowing whether something will be useful or not until implementing and using it anyway.

...it's tricky because the widget is rendererd through QTextDocument (header included), afaik I need to change the rendering code a lot in order to implement that.

In this case we could live without it then, putting significant effort into minor things like that would be a waste of time. What could be done instead, perhaps, is expanding the delegate API? Like letting it draw header's background, I am thinking of trying out painting some gradient there and seeing how it would look.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

But this may probably be the issue with separators themselves, drawn in the middle of empty characters instead of before or after them? I like separators drawn that way though, with some extra space to the left and right from them. So perhaps you could find a way for row colours, selection and metadata highlighting to take separators into account when being painted?

Now separators are drawn between two full characters, I have adjusted all QPointF <=> HexPosition conversion functions, I haven't noticed regressions.

Yea, why not, if it does not complicate the code. No reliable way of knowing whether something will be useful or not until implementing and using it anyway.

Added!

In this case we could live without it then, putting significant effort into minor things like that would be a waste of time. What could be done instead, perhaps, is expanding the delegate API? Like letting it draw header's background, I am thinking of trying out painting some gradient there and seeing how it would look.

Now QHexDelegate provides:

  • hexHeader: We have addressHeader and asciiHeader, so...
  • renderAddress: For example, to highlight the current address
  • renderHeaderPart: This highlights all three parts of the widget's header, if the label of the Hex/Ascii part is not overriden you have "00" "01" "02" ... for Hex and "0" "1" "2" "3" ... for Ascii provided as QString

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

I think the first argument of QHexView::dataChanged should be the QByteArray one, it is the most important out of them all, and that makes it possible to connect this signal to slots that do not require the two other parameters, e. g something like void processData (const QByteArray&).

Now separators are drawn between two full characters, I have adjusted all QPointF <=> HexPosition conversion functions, I haven't noticed regressions.

Almost perfect now, just a few more things:

  1. Metadata highlighting is misaligned slightly. It grabs an extra pixel from the address column (hard to see here, you may need to zoom in) and misses one empty character before the ASCII column. Should metadata highlighting paint the whole part of column that it works with, or not touch the empty separator characters from the left and right sides? I can not say which way would look better, so perhaps it could be determined via QHexOptions? One option for the Hex column and one for the ASCII, so it could be set separately for them.

Misalignment

  1. The end of ASCII area is cut off, look at the ASCII label in the screenshot below, only a small part of the last letter 'F' is seen.

  2. In the screenshot the size of QByteArray is 23 bytes. If you try to highlight more than that, e. g. call setBackground(0, 32, QColor::fromRgb(0xff9999)) it will grab an extra empty character. This does not happen if just the right length is supplied, that is setBackground(0, 23, QColor::fromRgb(0xff9999)). This would likely be the user's own fault to ask to highlight more than there already is, but nevertheless this could be fixed at least for the sake of consistency.

Extra character at the end

Now QHexDelegate provides...

Will take a look at it a bit later

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Should metadata highlighting paint the whole part of column that it works with, or not touch the empty separator characters from the left and right sides?

Like this?
Screenshot_20220309_153303

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Yea, that's option number 2. Number 1 is to highlight the empty characters next to separator lines as well. Hard to tell which one would look better, so this could be a choice via QHexOptions.

Edit: on the other hand these empty separator characters on the sides are probably supposed to separate only and not be a part of highlighting, so maybe just leave everything as it is on you screenshot, without these extra options I mentioned earlier.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Ok, I think that I have fixed everything.

In the screenshot the size of QByteArray is 23 bytes. If you try to highlight more than that, e. g. call setBackground(0, 32, QColor::fromRgb(0xff9999)) it will grab an extra empty character. This does not happen if just the right length is supplied, that is setBackground(0, 23, QColor::fromRgb(0xff9999)). This would likely be the user's own fault to ask to highlight more than there already is, but nevertheless this could be fixed at least for the sake of consistency.

I can't reproduce it anymore with the latest changes:
Screenshot_20220309_165126

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Ok, I think that I have fixed everything.

Indeed, but I caught a few more:

Header and selection

  1. Hard to notice, but on the left side of header there is an area one pixel wide which remains unpainted, despite the usage of delegate.

  2. Same on the right side, except it is a bit wider. Also shouldn't there be a whole empty character separating ASCII area from widget's right border, just like same empty character separates the address column from the widget's left border?

  3. Look at cursor's selection. It works as it is supposed to, not grabbing the empty space after the last character in selection. But in this particular case it erased a part of metadata highlighting (that white space in the middle is supposed to be green).

Now QHexDelegate provides...

Haven't used the functions you mentioned, but of course I welcome any new good public API, as I said before. Now that there are much more ways to customise QHexView, once the new version is finished the documentation would benefit from more examples with screenshots. What I used was the QHexDelegate::renderHeader to achieve effect in the above screenshot (this might also be suitable as one of the styling examples):

void renderHeader (QTextBlockFormat& bf, const QHexView* hex) const override
{   // Could be improved with colours taken from QPalette
    QLinearGradient gradient(0, 0, 0, QFontMetrics(hex->font()).height());
    gradient.setColorAt(0.1, Qt::white);
    gradient.setColorAt(1, Qt::gray);
    QBrush brush(gradient);
    bf.setBackground(brush);
}

You've done more than enough already when introduced QHexDelegate, but I am curious, could it be given access to QPainter pointer and header's rectangle, like Qt's own delegates have, without much effort? It could potentially open up even more styling customisations. For instance I used a small trick to draw the header style on the first column of QTreeWidget using QStyledItemDelegate, here's part of the QStyledItemDelegate::paint overriden function from it:

void paint (QPainter* painter, const QStyleOptionViewItem& option, const QModelIndex& index) const
{
    if (index.column() == 0)
    {   // QStyledItemDelegate::paint is not called at all
        QStyleOptionHeader headerStyleOption;
        headerStyleOption.text = index.data().toString();
        headerStyleOption.rect = option.rect;

        QWidget* parent = qobject_cast<QWidget*>(this->parent());
        if (parent)
        {
            parent->style()->drawControl(QStyle::CE_HeaderSection, &headerStyleOption, painter, parent);
            parent->style()->drawControl(QStyle::CE_ItemViewItem, &option, painter, parent);
            parent->style()->drawControl(QStyle::CE_HeaderLabel, &headerStyleOption, painter, parent);
        }
    }
}

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Hard to notice, but on the left side of header there is an area one pixel wide which remains unpainted, despite the usage of delegate.

I can't reproduce with the latest changes (see below).

Same on the right side, except it is a bit wider. Also shouldn't there be a whole empty character separating ASCII area from widget's right border, just like same empty character separates the address column from the widget's left border?

Fixed, I think that the address column and header should draw the separator's area too, for example (I have used your code):
Screenshot_20220310_103016
I can highlight the address column according to the cursor's position entirely! (at this point I'm thinking to restore the highlighting on the separator's area too, or leaving it as it is).

You've done more than enough already when introduced QHexDelegate, but I am curious, could it be given access to QPainter pointer and header's rectangle, like Qt's own delegates have, without much effort

Yes, I think we need beforePaint and afterPaint: the first one may be overwritten by the default paintEvent but it can be useful to draw backgrounds.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

I can't reproduce with the latest changes (see below).

My bad, it was just the style of QFrame's frame that I have changed to QFrame::Box | QFrame::Sunken

Fixed, I think that the address column and header should draw the separator's area too, for example (I have used your code):

The header is fine now, but there is still that small inconsistency with distance from border on the left and on the right. Look at the highlighted parts of ASCII area on your screenshot, they illustrate It better. The distance between them and scrollbar is slightly smaller compared to the distance between the 00000000, 00000010, etc. parts and the left border of the widget. On the left we've got a full character width separating addresses from widget's border, but on the right it seems there is like only half a character.

I can highlight the address column according to the cursor's position entirely!

Sounds like a good idea, could be added as a flag to QHexOptions.

I'm thinking to restore the highlighting on the separator's area too, or leaving it as it is

I think I like the way it is now on your screenshot, but if you go for it it make it an option to choose from.

I think we need beforePaint and afterPaint

Wouldn't a single paint function suffice? In Qt's widgets paint events and delegates it is up to user's code to call the paint function of superclass before or after the custom drawing is done.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

The header is fine now, but there is still that small inconsistency with distance from border on the left and on the right. Look at the highlighted parts of ASCII area on your screenshot, they illustrate It better. The distance between them and scrollbar is slightly smaller compared to the distance between the 00000000, 00000010, etc. parts and the left border of the widget. On the left we've got a full character width separating addresses from widget's border, but on the right it seems there is like only half a character.

Mmmh, I have noticed that, but afaik there aren't any half characters anymore, I don't know why it happens: the width is calculated as endColumnX + vscrollbar_width (if visible), endColumnX is the end of the horizontal separator.

Sounds like a good idea, could be added as a flag to QHexOptions.

Added as QHexFlags::HighlightAddress and QHexFlags::HighlightColumn

Wouldn't a single paint function suffice? In Qt's widgets paint events and delegates it is up to user's code to call the paint function of superclass before or after the custom drawing is done.

I've modified paintEvent in order to achieve this: QHexDelegate::paint can be called after or before your custom code

from qhexview.

neomissing avatar neomissing commented on July 17, 2024

Can support such display, no data does not show the number
20220310204051

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

I don't know why it happens

I will try to have a look, maybe I'll find something.

Could you introduce something like QHexDelegate::drawVerticalSeparators and QHexDelegate::drawHorizontalSeparator? I have found a way to draw the QFrame's VLine, but it required a few hacks. And also perhaps there should be some function that returns header's rectangle (below you may find how I acquire it, if this is correct you can use this method).

#include <qdrawutil.h>
virtual void CustomDelegate::paint (QPainter* painter, const QHexView* hex) const override
{
    // Paint header (not necessarily better than using renderHeader, just an example)
    QLinearGradient gradient(0, 0, 0, QFontMetrics(hex->font()).height());
    gradient.setColorAt(0.1, Qt::white);
    gradient.setColorAt(1, Qt::gray);
    painter->fillRect(QRect (0, 0, hex->width(), QFontMetrics(hex->font()).height()), gradient);
        
    // An ugly hack to remove vertical separators to paint custom ones
    QHexView* hacked = const_cast<QHexView*>(hex);
    QHexOptions original = hex->options();
    QHexOptions temporary = original;
    temporary.flags &= ~QHexFlags::VSeparator;
    hacked->setOptions(temporary);
        
    QHexDelegate::paint(painter, hex);
        
    // Had to make hexColumnX() and asciiColumnX() public for this
    qDrawShadeLine(painter, hex->hexColumnX(), 0, hex->hexColumnX(), hex->height(), hex->palette());
    qDrawShadeLine(painter, hex->asciiColumnX(), 0, hex->asciiColumnX(), hex->height(), hex->palette());
        
    hacked->setOptions(original);
}

The result is (background is set elsewhere):

Vline

Also it appears there is a small regression (or we simply have not noticed this earlier). In header, when the address label grows it pushes other labels to the right. Observe the number of byte position in the screenshot above (two digits), and then on the screenshot below (three digits):

Labels pushed

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Could you introduce something like QHexDelegate::drawVerticalSeparators and QHexDelegate::drawHorizontalSeparator? I have found a way to draw the QFrame's VLine, but it required a few hacks. And also perhaps there should be some function that returns header's rectangle (below you may find how I acquire it, if this is correct you can use this method).

Added as QHexDelegate::paintSeparator, I've also added headerRect, addressRect, hexRect and asciiRect methods in QHexView class.

Usage:

bool CustomDelegate::paintSeparator(QPainter* painter, QLineF line, const QHexView* hexview) const
{
    qDrawShadeLine(painter, line.p1().toPoint(), line.p2().toPoint(), hexview->palette());
    return true;
}

Also it appears there is a small regression (or we simply have not noticed this earlier). In header, when the address label grows it pushes other labels to the right.

Fixed, QFontMetrics::elidedText works in a strange way for this use case (it cuts one or two characters), I have implemented a custom function in order to truncate and elide a string.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Can support such display, no data does not show the number 20220310204051

I can add a byte mask, but you can't edit it from the hex part because they will not be displayed until the "unmask condition" is satisfied

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Added as QHexDelegate::paintSeparator, I've also added...
Fixed, QFontMetrics::elidedText...

Tested, working well.

About that issue with the right side of widget: it has definitely something to do with setting the auto-width option. If it is disabled the distance to the "end" is exactly one character width, as expected.

Edit: it looks like something is wrong on my end, a clean project with nothing but QHexView appears to work exactly as it is supposed to (although you had that issue too on your last screenshot). I will investigate further.

Edit 2: no, seems to be an actual bug related to auto-width. I have measured pixels and even in clean project distance was a few pixels off when it is on and correct when it is off.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

It looks like despite QHexView with auto-width setting its own size, Qt resizes the widget slightly. Subclass QHexView, reimplement the resizeEvent and observe:

#include <QDebug>
#include <QResizeEvent>

class Test : public QHexView
{
public:
    Test ()
    {
        setAutoWidth (true);
    }

protected:
    void resizeEvent (QResizeEvent* event) override
    {
        QHexView::resizeEvent (event);
        qDebug () << "Event width:" << event->size().width() << "Width:" << this->width();
    }
};

In my case this->width() was 1078, which is exactly the width of 77 cells (each 14 pixels wide in that particular case). But the event->size(), on the other hand, was 1076. Qt is somehow always slightly reducing QHexView's width. Sometimes it is just 2 pixels like in this case, barely noticeable, sometimes it appears it cuts more. I have tried changing the following to setFixedWidth and setMinimumWidth, but it does not seem to take any effect:

this->setMaximumWidth(m_autowidth ? std::ceil(this->endColumnX() + vw) : oldmw);

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Could be a viewport issue too. setAutoWidth is set to true, font is increased to force the horizontal scrollbar, thus cell size is larger, but the distance from the border is still small (the horizontal scroll bar is scrolled maximum to the right, the empty space is actually occupied by the scrollbar pointer, it just hides itself on Windows 11 when mouse is not hovering over it).

Viewport issue

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

I guess I found a sort of solution. It involves... adding three pixels to the width. No idea why, and I am not a fan of adding some hardcoded values, but it seems to look consistent now, and no matter what the font size is.

Without:
Without 4 pixels

With:
With 4 pixels

If more than three are added it does not draw anything beyond that:
More

QHexView::checkState needs to be modified in two places:

this->setMaximumWidth(m_autowidth ? std::ceil(this->endColumnX() + vw) : oldmw);

this->setMaximumWidth(m_autowidth ? std::ceil(this->endColumnX() + vw + 3) : oldmw);

this->horizontalScrollBar()->setRange(0, std::max<int>(0, this->endColumnX() - this->width() + vw));

this->horizontalScrollBar()->setRange(0, std::max<int>(0, this->endColumnX() - this->width() + vw + 3));

Check if it works for you too, to make sure that monitor resolution (mine is 2880x1800) does not play any role here. Maybe you will find a deeper cause of such behaviour.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Yes, it looks fine on me too, pushed.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Probably one last thing I would like to ask for is a widget that complements the search and replace functionality. Otherwise everyone would just have to make their own, reinventing the wheel over and over again. This could be a part of qhexutils.

How sophisticated should this widget be? That I do not know, but it should have at least the rudimentary functionality of looking for ASCII text (does unicode require any extra effort or Qt already has it covered?) and byte sequences (probably with an option to mix them together in a single search expression).

More advanced features could include byte sequences with unknown data like 00 AB ?? ?? ?? CD EF. If an unknown part is too long maybe the expression could contain something like this 00 AB Skip 9 CD EF.

However that would require additional effort of creating a small domain-specific language, writing a parser, checking for errors and converting the result into some C++ code. Perhaps there already is some library that does that?

This is how such widget looks in the old HxD hex editor:

Hex search 1
Hex search 2
Hex search 3
Hex search 4

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Two small bugs:

  1. Horizontal scrollbar sometimes does not appear when supposed to. On the screenshot you can see that ASCII character 66 ('f') was selected, yet it is almost out of the view. The only way to see it is tor resize the widget. Auto-width is enabled, if that matters.

Horizontal scrollbar issue

  1. The QString::isEmpty call needs to be replaced with QString::isNull, at least for the second one. The reason is that if one wants the ASCII header not to be shown and sets hex options to an empty string like this options.asciilabel = "" the default ASCII string will still be displayed. The default constructor of QString creates a null one (both QHexOptions and QHexDelegate create a null QString), so it is a good way to distinguish between the "user did not touch this" and "user does not want this".

QHexView/qhexview.cpp

Lines 489 to 493 in 69dcb8f

if(asciilabel.isEmpty()) asciilabel = m_options.asciilabel;
if(asciilabel.isEmpty())
{
c.insertText(" ", { });

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

More advanced features could include byte sequences with unknown data like 00 AB ?? ?? ?? CD EF. If an unknown part is too long maybe the expression could contain something like this 00 AB Skip 9 CD EF.
However that would require additional effort of creating a small domain-specific language, writing a parser, checking for errors and converting the result into some C++ code. Perhaps there already is some library that does that?

I can port pattern based hex-search from REDasm (another project created by me):
https://github.com/REDasmOrg/REDasm-Library/blob/master/rdcore/support/utils.cpp#L73

There is also some WIP code for the search dialog in the repo now, search dialogs is an optional feature for CMake, but it's always enabled for QMake (I'm a CMake user, I don't know how to make it optional with QMake).

The QString::isEmpty call needs to be replaced with QString::isNull, at least for the second one. The reason is that if one wants the ASCII header not to be shown and sets hex options to an empty string like this options.asciilabel = "" the default ASCII string will still be displayed. The default constructor of QString creates a null one (both QHexOptions and QHexDelegate create a null QString), so it is a good way to distinguish between the "user did not touch this" and "user does not want this".

I wasn't aware of this slight difference: I have replaced isEmpty() with isNull() for the hexlabel too.

Horizontal scrollbar sometimes does not appear when supposed to. On the screenshot you can see that ASCII character 66 ('f') was selected, yet it is almost out of the view. The only way to see it is tor resize the widget. Auto-width is enabled, if that matters.

I will check it later!

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

I can port pattern based hex-search from REDasm (another project created by me):
https://github.com/REDasmOrg/REDasm-Library/blob/master/rdcore/support/utils.cpp#L73

If you already have it working then of course, go for it. Is there any documentation describing its syntax?

The byte pattern search has multiple applications, and I'd say it could be its own small standalone library. That code is licensed under LGPL though, could you give this particular byte pattern piece of code same license as QHexView has? I need to link statically to commercial software, and LGPL pretty much forces dynamic linking in that scenario.

There is also some WIP code for the search dialog

Will take a look at it later.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

That code is licensed under LGPL though, could you give this particular byte pattern piece of code same license as QHexView has? I need to link statically to commercial software, and LGPL pretty much forces dynamic linking in that scenario.

No worries, that piece of code is written by 100% by me: if I use some of my code from another project in QHexView will always be MIT licensed.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

if I use some of my code from another project in QHexView will always be MIT licensed.

Much appreciated!

The bugs of search dialog so far:

  1. Text search is case sensitive no matter what;
  2. When switching to integer search and attempting to switch to some other mode a crash occurs;
  3. I expected the find button to jump to the next occurrence of text, but it stops on the first one;
  4. Hexadecimal search is all wrong, I entered a two-digit hex number and it seems to just jump back and forth between two random points when the 'find' button is pressed;
  5. Integer search was unable to find an unsigned byte with a value of 1. Looks like it is unable to find anything at all yet.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Yes, it's still WIP:

  1. Search options wasn't implemented, try now
  2. I will investigate it
  3. Incremental search works now
  4. Fixed, you can also use patterns like aa bb cc ?? 00 11 22 ?? ?? ff 33 or aabbcc??001122????ff33, whitespaces are ignored
  5. Integer search is not yet implemented

EDIT:
Currently the "All" search direction is not implemented, the "Backward" one works only for text.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

...still WIP

That's okay, I could test immediately or wait for the new batch of changes, either way just let me know when you think it is more convenient to perform another round of testing.

I can confirm that 1, 3, 4 are fixed.

  1. In hexadecimal search, when using spaces in byte pattern an extra byte is selected upon successful search. My search pattern was three bytes long (11 22 33). Oddly enough if one space is removed and the other is still present (does not matter which one) the extra redundant byte is not selected.

  2. Tried backward search on text, it works in a strange way: if started at the beginning it jumps to the last occurrence. If started from the end (after the last occurrence) it is unable to find anything.

  3. Should patterns like this be considered incorrect: ?1, 1? ? Should the odd number of characters be considered as incorrect input too, e. g. 1A 2B 3 ? If so, then there needs to be a special error message instead of the default 'Can not find X'

  4. This is not really a bug, but more like the default behaviour that should probably be changed: when focus is switched from QHexView to something else, both cursors become invisible. The only way to make them visible again without also changing their position is to click on the header. Make them to be visible no matter what?

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

6. In hexadecimal search, when using spaces in byte pattern an extra byte is selected upon successful search. My search pattern was three bytes long (11 22 33). Oddly enough if one space is removed and the other is still present (does not matter which one) the extra redundant byte is not selected.

Fixed, there is still one issue when the selection length is 1, but it's a renderer issue, I will check that

Tried backward search on text, it works in a strange way: if started at the beginning it jumps to the last occurrence. If started from the end (after the last occurrence) it is unable to find anything.

I have improved the backward search, try now.

Should patterns like this be considered incorrect: ?1, 1? ? Should the odd number of characters be considered as incorrect input too, e. g. 1A 2B 3 ? If so, then there needs to be a special error message instead of the default 'Can not find X'

Yes I can add that

This is not really a bug, but more like the default behaviour that should probably be changed: when focus is switched from QHexView to something else, both cursors become invisible. The only way to make them visible again without also changing their position is to click on the header. Make them to be visible no matter what?

Done: now cursors are drawn using QPalette::Disabled when the widget is not focused

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

Fixed, there is still one issue when the selection length is 1, but it's a renderer issue, I will check that

Everything appears to work correctly, except for the case when search pattern consists of a single byte. Is that what you meant? It finds the first occurrence of that byte and then stops searching altogether, not even showing the "Cannot find" message after many clicks on the "Find" button. I have not checked the code but my guess would be that it keeps finding that same byte over and over again at that position without moving on.

I have improved the backward search, try now.

Better now, but it looks like it acts the same way the single byte search does, as described above.

Done: now cursors are drawn using QPalette::Disabled when the widget is not focused

Confirmed, works correctly.

And one more: this does not work: hex_view->setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOn)

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Everything appears to work correctly, except for the case when search pattern consists of a single byte. Is that what you meant? It finds the first occurrence of that byte and then stops searching altogether, not even showing the "Cannot find" message after many clicks on the "Find" button. I have not checked the code but my guess would be that it keeps finding that same byte over and over again at that position without moving on.

Fixed

Better now, but it looks like it acts the same way the single byte search does, as described above.

Fixed

And one more: this does not work: hex_view->setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOn)

It was handled by the widget, it should work now

In general search/replace feature is completed, it needs some testing for bugs or regressions

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

The text and hexadecimal searches seem to be completed, but the integer and float ones need more work, and also the replacing functionality:

  1. A crash still occurs when trying to switch back from integer search (just this one, all others can be switched without problem);

  2. In the integer search when negative value is written the "Find" button becomes disabled;

  3. In the integer search when choosing direction type "All", and searching for anything other than single byte value, the search does not loop back like it does in text and hexadecimal searches;

  4. There is no endianness option in the float search;

  5. The float search, in fact, does not appear to work at all. I have tried searching for the value of 1 (00 00 80 3F, right?) in both endianness but it could not find it;

  6. When using hexadecimal replace a crash occurs when: 1) There are no more patterns to replace 2) "Replace" button is pressed;

  7. Hexadecimal replace input does not support the ?? syntax;

  8. There is no "Replace all" option or button.

Other notes:

I think #70 may be be fixed now, although I have never used the API directly, but when forward and backward searches are used from the search/replace widget they work as expected (for text and hexadecimal search at least).

Have you decided whether adding syntax like this 00 AB Skip 9 CD EF to the byte pattern search is worth it? I understand it may be tricky and error-prone, but if there is a simple solution it could be a useful addition.

There is one bug, which is very likely just a bug in my own software (maybe even a bug of Qt 5.14.2), but since it happens to occur with QHexView I thought I'd report it. I can not even reproduce it in a simple test myself, it happens only in my program, so it is unlikely this is related to QHexView, I am just posting it in case you accidentally come across anything like this too. I have even looked through the related QHexView's code and could not find anything wrong. The bug is:

  1. Some default value for the point size of font is set in the constructor of QHexView's subclass (QWidget::setFont);
  2. A different font variable for an instance of that subclass is set elsewhere later (same font, just point size changed);
  3. A spurious QEvent::FontChange is sent to QHexView with the old font, changing it back to whatever was set in constructor before.

Happens during initialisation only, later I can change font back and forth via GUI just fine.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024
  1. Bug: vertical scrollbar does not appear during a certain circumstance. Take a look at the first screenshot, notice the last line full of FF bytes:

FF seen

Now look at the same widget which width is reduced. The horizontal scrollbar appeared as it was supposed to. However, it obscured the last line with FF bytes. Vertical scrollbar is supposed to appear in such case, but it didn't, thus the last line is inaccessible.

FF not seen

  1. I came up with two byte pattern match/replace functions, maybe they can be useful here as well. std::variant helps a lot, allowing to store either a byte sequence to match for, or the number of bytes to skip to the next sequence. All you need to do is to convert the 00 AB CD EF string to this pattern.

std::variant was added in C++17 though, and since you mentioned that you would like to keep C++11 compatibility, this could be problematic. Unless there is a really good reason not to, I'd say stick with C++17. After all, if anyone needs to compile on an older system installing a newer compiler should not be a huge problem.

#include <variant>
#include <QDebug>

using BytePatternUnit = std::variant<QByteArray, int>;
using BytePattern = QVector<BytePatternUnit>;

bool bytePatternMatch (const QByteArray& data, const BytePattern& pattern)
{
    int offset = 0;
    for (const BytePatternUnit& unit : pattern)
    {
	if (std::holds_alternative<QByteArray>(unit))
        {
	    QByteArray array = std::get<QByteArray>(unit);
	    if (data.indexOf(array, offset) == -1)
	        return false;
            else
	        offset += array.size();
        }
        else if (std::holds_alternative<int>(unit))
        {
	    offset += std::get<int>(unit);
        }
    }
    return true;
}

void bytePatternReplace (QByteArray& data, const BytePattern& pattern)
{
    int offset = 0;
    for (const BytePatternUnit& unit : pattern)
    {
        if (std::holds_alternative<QByteArray>(unit))
        {
	    QByteArray array = std::get<QByteArray>(unit);
            data = data.replace(offset, array.length(), array);
	    offset += array.length();
        }
        else if (std::holds_alternative<int>(unit))
        {
	    offset += std::get<int>(unit);
        }
    }
}

int main ()
{
    QByteArray data ("Hello, world!");
    BytePattern pattern_01 = {"Hello", 2, "world!"};
    qDebug () << "Pattern matched:" << bytePatternMatch (data, pattern_01);
    BytePattern pattern_02 = {7, "there!"};
    bytePatternReplace (data, pattern_02);
    qDebug () << "Data modified:" << data;
    return 0;
}
  1. In the source code you can safely replace the Q_EMIT macros with the 'emit' keyword. If you are worried about the QT_NO_KEYWORDS macro, then fear not:
  • They are in .cpp files, so they are not supposed to be affected;
  • There is QT_NO_SIGNALS_SLOTS_KEYWORDS alternative introduced in Qt5 which disables the 'signals' and 'slots' keywords, but leaves the 'emit' one.
  1. The binary data that QHexView::dataChanged signal now delivers is a copy of a small part that was edited via GUI, instead of the whole byte array. This is not a problem of course, in fact this was an excellent decision. In my particular case though I deal with small blobs of QByteArrays, so it is easier for me to just send a copy of the whole newly edited binary data. This is very easy of course, given that this signal also provides the offset and operation type. But I was wondering, perhaps emitting the signal with a whole copy of binary data from QHexView could be an optional feature? It could be a second signal, disabled by default and activated only if some flag in QHexOptions is set. I myself not sure whether my particular case is worthy of being taken care of though (especially since it took me less then 10 lines of code to deal with it myself), so I do not insist here.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Bug: vertical scrollbar does not appear during a certain circumstance. Take a look at the first screenshot, notice the last line full of FF bytes

It can be a rounding issue with the viewport, I will check it when I have some free time

I came up with two byte pattern match/replace functions, maybe they can be useful here as well. std::variant helps a lot, allowing to store either a byte sequence to match for, or the number of bytes to skip to the next sequence. All you need to do is to convert the 00 AB CD EF string to this pattern.
std::variant was added in C++17 though, and since you mentioned that you would like to keep C++11 compatibility, this could be problematic. Unless there is a really good reason not to, I'd say stick with C++17. After all, if anyone needs to compile on an older system installing a newer compiler should not be a huge problem.

I use C++17 too, but usually I try to keep C++11 compatibility with libraries, can it be implemented with QVariant?

  1. The binary data that QHexView::dataChanged signal now delivers is a copy of a small part that was edited via GUI, instead of the whole byte array. This is not a problem of course, in fact this was an excellent decision. In my particular case though I deal with small blobs of QByteArrays, so it is easier for me to just send a copy of the whole newly edited binary data. This is very easy of course, given that this signal also provides the offset and operation type. But I was wondering, perhaps emitting the signal with a whole copy of binary data from QHexView could be an optional feature? It could be a second signal, disabled by default and activated only if some flag in QHexOptions is set. I myself not sure whether my particular case is worthy of being taken care of though (especially since it took me less then 10 lines of code to deal with it myself), so I do not insist here.

It's easier to add another signal like fullDataChanged so it's always there when needed!

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

... can it be implemented with QVariant?

Yea, and almost identically in fact: the pattern becomes a simple QVariant sequence, then just replace std::holds_alternative checks with QVariant::canConvert and std::get with QVariant::value. And I suppose a final else statement needs to be added to deal with the case when something other than QByteArray or int accidentally got into the pattern to abort operation, exception could be thrown from there then.

It's easier to add another signal like fullDataChanged...

Pretty much, if it does not mess up the design it would be a good addition. And it could probably be emitted optionally, I imagine if someone happens to deal with very large quantities of data they would not want gigabytes copied around via such signal.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

I can take a look this weekend.

Yes, I can push 5.0 to master branch after these fixes.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

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:

15 6D 6F B5 0E 50 04 38 B0 82 51 17 C5 20 3C 25

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 QActions I suppose, or some other option which controls what CTRL + C produces by default). That is, outputting this:

{0x15, 0x6D, 0x6F, 0xB5 ...} or this "\x15\x6D\x6F\xB5 ..."

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 QAction or function to convert these bytes into a custom string.

And finally the user needs to be given control over whether this string created via the CTRL + C combination will occupy a single line or multiple lines just like in the QHexView itself.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

Ok so:

  • std::isprint() is replaced with QChar::isPrint()
  • I have added QHexView::copyAs(CopyMode mode) slot which copies data in various formats:
    • QHexView::CopyMode::HexArrayChar: data is copied as "\x15\x6D\x6F\xB5 ..."
    • QHexView::CopyMode::HexArraySquare: data is copied as [0x15, 0x6D, 0x6F, 0xB5 ...]
    • QHexView::CopyMode::HexArrayCurly: data is copied as {0x15, 0x6D, 0x6F, 0xB5 ...}

Also earlier we were talking about the possibility of introducibg QHexView::fullDataChanged signal as an optional alternative to the QHexView::dataChanged one, but now I am not that sure if it would be that useful, at least I myself in my current project find the granular approach of QHexView::dataChanged superior. Although perhaps someone else could find it useful, so if it does not mess up the design it could still be optionally included.

Ok, I prefer to keep only QHexView::dataChanged signal for now.
A "possible workaround" is to ignore the signal's arguments and extract the internal buffer with QHexDocument (it depends how the bytes are loaded though)

And finally the user needs to be given control over whether this string created via the CTRL + C combination will occupy a single line or multiple lines just like in the QHexView itself.

Mmmmh, for example?

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

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 QHexOptions, or a second optional parameter of QHexView::copyAs). With that, in the example above, instead of a single long line we have 8 short ones which makes it much more readable.

And I suppose this feature of inserting new line characters should itself be optional (a boolean value in QHexOptions I suppose). For example, if 17 bytes need to be copied instead of 128 we would not want to reserve a whole new line just for that last byte. Also we can never know where exactly the byte string will be pasted and what are the requirements of the software or file format where it gets pasted, so we can never assume which is the right choice there. However, I think for CopyMode::Visual the default result should be a single line string, and multi line string for others.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

I have added QHexOptions::copybreak (default true) which breaks at QHexOptions::linelength.

CopyMode::Visual behavior is now changed: it copies data as they appears in the hex part.
For example, if QHexOptions::grouplength is 4, data is copied as:

0x61736461, 0x73646173, 0x64206173, 0x64613334, 
0x35617364, 0x730A

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

I checked it out, working smoothly. A few comments:

  • QHexView::copyAs has two unused i variables:

QHexView/qhexview.cpp

Lines 261 to 263 in 5105af5

case CopyMode::HexArrayChar: {
QString hexchar;
int i = 0;

QHexView/qhexview.cpp

Lines 272 to 274 in 5105af5

default: {
QString hexchar;
int i = 0;

  • Perhaps the CopyMode::HexArrayChar and the regular QHexView::copy function could also benefit from the QHexOptions::linelength option for line breaks?

  • There is one more hex notation that I am aware of, in Common Lisp byte vectors are created this way:
    #(#xFF #xFF #xFF #xFF). Perhaps QHexView::copyAs could be made more generic to allow a wider range of customisation. For instance, two extra options could be added in QHexOptions, like this:

    QString hex_prepend = "0x"
    bool add_comma = true or even better QString hex_separator = ", "

    And I suppose since we already have got the CopyMode::HexArrayCurly and CopyMode::HexArraySquare why not add the third bracket type, which would be the CopyMode::HexArrayParentheses. In fact these three enumerations could possibly themselves be moved to QHexOptions, and the QHexView::CopyMode would need to contain only thee values - Visual as default, WithBrackets to wrap the result of Visual in brackets, and HexArrayChar as special case. In that case, perhaps CopyMode::Visual could be renamed to CopyMode::Default?

    All that combined would make it possible to customise the copy output for pretty much any programming language.

    To sum up, this is what the code could possibly look like:

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;
    };
};
  • I have noticed that when QHexView contains no data, the cursors are still drawn along with the 00000000 position indicator. I think it would look nicer if it does not happen.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

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 FF bytes can not be fully seen. If I slightly adjust the widget's height I am able to fully see it again. If I keep changing height this issue comes and goes at various height values.

Last line obscured

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

The last commit seems to fix this issue (on linux).
Now visible lines are calculated with the viewport's height instead of the widget one

I see in the next days this one: #71 (comment)

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

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 QApplication::setAttribute (Qt::AA_EnableHighDpiScaling) is called. The resolution of display I work with is 2560 × 1600. I work under Windows but this might be irrelevant in this case.

If QApplication::setAttribute (Qt::AA_EnableHighDpiScaling) is not called on this system the widget works fine. Unfortunately setting this attribute is mandatory because without it many widgets look incorrectly on displays with high resolution.

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:

Normal

I keep extending height and then some extra space appears (is this intentional?) :

Extra space

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.

Partially obscured

The cycle repeats as I keep increasing the height of widget.

One a side note, the issue that was fixed when std::isprint() was replaced with QChar::isPrint() was related to high DPI mode as well, but in this case it appears to be a bug in Qt itself (at least version 5.14.2) and only on Windows platform in particular. This CreateFontFaceFromHDC() failed error occurs every time Qt attempts to draw non-printable symbols on Windows system in High DPI mode. I have observed it when some non-printable characters ended up going into QPlainTextEdit and when simply selecting some fonts from the QFontDialog::getFont call. So this is the right decision to replace all non-printable characters with dots, make sure it stays that way.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

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 vscrollmax somewhere in QHexView::checkState, before calling the setRange of the scrollbar:

QHexView/qhexview.cpp

Lines 406 to 413 in 67dc02e

void QHexView::checkState()
{
if(!m_hexdocument) return;
this->checkOptions();
int doclines = static_cast<int>(this->lines()), vislines = this->visibleLines(true);
qint64 vscrollmax = doclines - vislines;
if(doclines >= vislines) vscrollmax++;

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.

Extra space

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.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

If a +1 improves the situation it can be a rounding issue where the line's height is calculated, maybe a qCeil() fixes that.

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

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 QPlainTextEdit despite setting a monospace font. Further investigation showed that 4 spaces were not equal in width to 1 tab. As usual the widget worked fine when this mode was disabled.

As it turned out, the default tab width value was an integer value of 32 and had to be recalculated with the help of QFontMetricsF to become 31.xxxx.

I see that you are already using QFontMetricsF and qreal a lot, but perhaps there is still some place where integers are used instead, which is causing the issue?

from qhexview.

arabine avatar arabine commented on July 17, 2024

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?

from qhexview.

arabine avatar arabine commented on July 17, 2024

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).

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

5.10 is old enough I believe

The goal is to retain compatibility with the version 5.6.3 though, according to this discussion - #69

from qhexview.

NikolajSchlej avatar NikolajSchlej commented on July 17, 2024

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:

void QHexView::wheelEvent(QWheelEvent* e)
 {
     e->ignore();
 #if defined Q_OS_OSX
     // In macOS scrollbar invisibility should not prevent scrolling from working
     if(!m_hexdocument) return;
 #else
     if(!m_hexdocument || !this->verticalScrollBar()->isVisible()) return;
 #endif

from qhexview.

NikolajSchlej avatar NikolajSchlej commented on July 17, 2024

Also, the default font in macOS is somehow not a monospace one, so I had to manually call setFont() to set it back to sanity. This is how it looks in macOS 13.3.1 on the same 5K screen.
Screenshot 2023-04-23 at 16 11 37
Screenshot 2023-04-23 at 16 12 03

from qhexview.

T-640 avatar T-640 commented on July 17, 2024

@NikolajSchlej,

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 QApplication::setAttribute (Qt::AA_EnableHighDpiScaling) in advance before the QApplication instantiation and pay close attention to the last line, as the glitch may be hard to spot. A somewhat crude workaround can be found here - #71 (comment).

The reason behind it is yet to be determined, so any assistance would be appreciated.

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

@arabine

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).

There is QHexDocument::documentChanged() signal which is connected automatically to the view and it redraws the data when notified.

If data is changed externally I think it's sufficient to trigger an update manually with Qt's update() method when needed

from qhexview.

NikolajSchlej avatar NikolajSchlej commented on July 17, 2024

@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)".
Will still try to reproduce it on supported configurations later.

from qhexview.

NikolajSchlej avatar NikolajSchlej commented on July 17, 2024

@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.

Screenshot_20230424_112306

from qhexview.

arabine avatar arabine commented on July 17, 2024

@Dax89

There is QHexDocument::documentChanged() signal which is connected automatically to the view and it redraws the data when notified.

If data is changed externally I think it's sufficient to trigger an update manually with Qt's update() method when needed

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).
It would be worth to have it, it would complete the README examples provided (the examples show only the use case when the user can change the data from the view).

from qhexview.

Dax89 avatar Dax89 commented on July 17, 2024

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.
Note that QMemoryRefBuffer have deletion and insertion disabled, but you can edit its content.

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.

from qhexview.

caffedrine avatar caffedrine commented on July 17, 2024

Guys, is there any way to scroll to a highlighted area? I tried manually method QHexView::scroll(0, 20) and it does produce a really weird output. It seems the whole area is shifted down:
image

from qhexview.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.