Issue #3 resolved

[hachoir-wx] Slow field redraw

Gentaro Muramatsu
created an issue

When I tried to open an MP3 file with hachoir-wx (on Windows platform), I was irritated with freezing window during update of thousands of fields.

I created a patch to make the fields list control virtual mode. This patch improves performance of field update especially when the file has many fields inside.

Comments (8)

  1. Victor Stinner repo owner

    Hi, a patch for hachoir-wx. Cool :-) But I'm unable to review the patch. I should ask hachoir-wx's authors.

    You patch doesn't apply correctly on the last Mercurial version. Could you please redo your patch on the last development version?

  2. Anonymous

    I really like this patch! It makes hachoir_wx much more usable. Some stylistic notes:

    Can you use 4-space indents instead of 2 spaces? This will make the code more readable, as the rest is with 4 spaces. Don't put spaces between the brackets and the arguments (e.g. use (a, b, c) instead of ( a, b, c )). Try to avoid the very confusing notation (self.has_parent and 1 or 0 ); use a counter variable and add one to it if there is a parent. assert( 0 <= col and col <= 5 ) can be shortened to assert(0 <= col <= 5)

    Comments on the patch: The result hack, using a list to pass data, seems very strange. I understand the dispatcher mechanism does not allow values to be passed directly, but is there perhaps a more standard way of doing this. If you can't find a better solution, document the problem in comments so that it is clear what you are doing.

    The autosizing columns results in some strange behaviour when there is no data in the column. It should employ a minimum width which fits the header.

    Can the hex_view change be moved to a new patch? It's not at all related to the current code, and I don't think it brings any major performance benefit (if indeed that was your goal).

    If you can address these points, I think we can accept the patch. It is very useful, thank you!

  3. Gentaro Muramatsu reporter

    I updated my patch. The attachment is a patch against revision 1131.

    Can you use 4-space indents instead of 2 spaces? Don't put spaces between the brackets and the arguments

    I modified my code to follow these rules.

    The result hack

    Now callback routine is registered in "on_field_view_ready". And the callback routine in field_view_imp_t is called with no assistance of the dispatcher.

    The autosizing columns results in some strange behaviour when there is no data in the column.

    Now it is ensured that column captions are visible.

    And I fixed a bug, which was mixed in my previous patch, that address mode switch didn't work. (self.format_addr shall be called indirectly.)

  4. nneonneo

    The callback looks great. I've made some minor adjustments to the patch, but nothing major. Can you check to make sure this is fine with you? Then I think it can be committed.

    I've tested the patch on Windows (I don't have access to a working Linux machine) and it works correctly.

  5. Log in to comment