Dax89 / QHexView

A versatile Hexadecimal widget for Qt5
MIT License
323 stars 101 forks source link

Color and Comment misalignment #79

Open Duolabs opened 3 months ago

Duolabs commented 3 months ago

Hello everyone, and congratulations on this viewer, which is among the best I have seen for QT. I am facing a series of problems that I cannot understand if they are due to incorrect use on my part, a bug, or simply the lack of the function. Here is what happens to me:

I initialize the memory of the buffer that I want to add to the document and add it to the last bytes of the document:


QByteArray PackedData(in_Buffer, in_Bytes);
g_hexdocument->insert(g_hexdocument->length(), PackedData);

I add a comment and a color to the text:

g_hexview->setForeground(StartPosition, EndPosition, TextColor);  // Set Text Color
g_hexview->setComment(StartPosition, EndPosition, Comment);  // Add a comment at offset range [12, 42)

I add more and more packets.

Now I remove the very first packet from the document:

g_hexview->setComment(0, in_Bytes, "");  // Clear Comment
g_hexdocument->remove(0, in_Bytes);

So far, no problem, the editor removes everything correctly from the QHexDocument.

The problem is that if I remove bytes from position 0 (in my case I have a FIFO that displays a rotating buffer) the color remains offset if the packets are of different sizes.

In essence, while the data is processed correctly, colors and comments remain fixed and are not removed and remapped correctly for the remaining data. The result is that comments and colors always refer to the positions of QHexView and are not anchored to QHexDocument.

Unfortunately, I believe this is the intended behavior, which creates significant problems in the case of dynamic editing with a rotating buffer.

The question is if I have correctly understood the operation, if I am using the objects correctly, or if it is simply a limitation of the code.

Using a FIFO in a data stream becomes too slow to keep the map of all packets and dynamically change comments and colors on the fly every time the oldest packet is removed.

Any help would be greatly appreciated.

Duolabs commented 3 months ago

No clue guys?

Dax89 commented 3 months ago

I'm on summer holidays, I can't check it right now!

Dax89 commented 3 months ago

Ok, I have checked the code, and yes: metadata informations are a model, so they are retained between data changes it's the intended behavior.

What behavior do you want to achieve instead? All metadata related to a specific range must be deleted when something changes? Or something else?

Duolabs commented 3 months ago

Hi Dax89,

Thank you for your answer and sorry for the delay, but I also took a few days off. In the meantime, I’ve tried and reviewed the code myself and have come to the conclusion that if you remove bytes that have a comment or a color, the related metadata doesn't change, which results in everything being misaligned.

Let me give you an example:

  AA BB CC DD    00 01 02 03
|   comment1  | | no comment |

If you remove AA BB CC DD, you end up with comment 1 at position 0, where AA BB CC DD no longer exists, but 00 01 02 03 is there instead. So the appropriate comment for the original bytes is now "assigned" to the wrong data.

This is what happens on my end, and I believe it's what you're referring to in your response. As you can see, it would be correct for the metadata to follow the bytes in the buffer, and therefore, in this case, be deleted as well. Does that make sense? Let me know if what I’m seeing here aligns with the correct functioning of the library.

P.S. I've added a few changes to the code, for example, to increase the font size when you press the CTRL key and move the mouse wheel, similar to other editors.

Note: I've noticed that when using certain fonts, the separator lines become misaligned with the text and sometimes end up almost in the middle of the hexadecimal bytes.

Dax89 commented 2 months ago

Ok, it's clear.

I can add that feature, but it can be very slow because the widget needs to check where the insertion/deletion happens and move every metadata that follows by the same amount of line/columns in order to keep visualization consistent.

Metadata is stored internally as map of lists of ranges where the key is the line (so it's not flat).

There is (a better) alternative: you can create you own QHexDelegate, with this class you can manage bytes' highlighting everytime a repaint happens.

class MyHexDelegate: public QHexDelegate {
    Q_OBJECT

public:
    ...
    bool render(quint64 offset, quint8 b, QTextCharFormat& outcf, const QHexView* hexview) const {
      // You can handle highlighting here
    }
    ...
};

// Attach the delegate to QHexView instance
auto* mydelegate = new MyHexDelegate(g_hexview);
...
g_hexview->setDelegate(mydelegate);
...

I can extend QHexDelegate it order to support comments too, in this case you keep metadata information on your side and it will be served to QHexView when needed.