Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

Minimap for hexedit #266

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Minimap for hexedit #266

wants to merge 8 commits into from

Conversation

adamiwaniuk
Copy link
Contributor

@adamiwaniuk adamiwaniuk commented Jul 5, 2017

This change is Reviewable

@mkow mkow changed the title minimap enabled Minimap for hexedit Jul 5, 2017
Copy link
Contributor

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Please make it visible by default
  • When both minimap and node tree are open resizing minimap is not too convenient (because it resizes both panels).

@@ -48,10 +48,11 @@ class VisualizationMinimap : public QOpenGLWidget,
ENTROPY
};

explicit VisualizationMinimap(QWidget *parent = 0);
explicit VisualizationMinimap(bool size_control = true, QWidget *parent = 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parent should be the first argument (same as you did in MinimapPanel ctor)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it is wrong in MinimapPanel , all qt things have it as last argmunet

@@ -62,6 +62,10 @@ HexEditWidget::HexEditWidget(MainWindowWithDetachableDockWidgets *main_window,
connect(hex_edit_, &HexEdit::selectionChanged,
this, &HexEditWidget::selectionChanged);

connect(hex_edit_, &HexEdit::visibleRegionChanged, [this] (qint64 start_addr, qint64 region_size) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no space after ]

// show_minimap_act_->setChecked(false);
// connect(show_minimap_act_, SIGNAL(toggled(bool)), this,
// SIGNAL(showMinimap(bool)));
//Currently not implemented
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after //

// SIGNAL(showMinimap(bool)));
//Currently not implemented
show_minimap_act_ = ShortcutsModel::ShortcutsModel::getShortcutsModel()->createQAction(
util::settings::shortcuts::SHOW_MINIMAP,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent with 4 spaces

@@ -44,6 +44,8 @@
namespace veles {
namespace ui {

const float k_minimap_selection_factor = 10;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consts should be in uppercase

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convention from this component, see trigram.cc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll refactor this old convention in few days (after fixing Jenkins), so insert new code in Google C++ Style

size_t minimap_size = std::min(static_cast<size_t>(selection_size * grow_factor), full_size);
minimap_sizes.append(minimap_size);

size_t offset = start_position % minimap_size;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will crash for an empty file


size_t offset = start_position % minimap_size;

// jump to begin if selection overflows bar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

begin -> beginning
jump -> Jump
bar -> bar.

offset = 0;
}

// fit end of bar to end of data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fit, data.

} while (selection_size < full_size);

int index = 0;
while(index < minimap_sizes.size()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after while

do {
selection_size = std::min(full_size, selection_size);
minimap_selection_sizes.append(selection_size);
size_t minimap_size = std::min(static_cast<size_t>(selection_size * grow_factor), full_size);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIR we use uint64_t in other places for file pos, this won't work well on 32-bit. Please fix this also in other places in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no for minimaps size, see other methods in this file

@adamiwaniuk
Copy link
Contributor Author

adamiwaniuk commented Jul 5, 2017

I will prefer to not enable this by defualt:

  • it is not tested well yet
  • since it uses GL it can cause some problems when somebody don't have graphic card

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants