Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graph scrolling forever #1195

Merged
merged 19 commits into from
Feb 16, 2019
Merged

Conversation

Vane11ope
Copy link
Contributor

Graph now scrolls forever and Overview also applies for it.

even when only a node is shown on the screen, you can move the node.

Also refactoring a lot of Graph and Overview code.

gifrecord_2019-02-14_202137
gifrecord_2019-02-14_201737

@ITAYC0HEN
Copy link
Member

Can't check until agJ would be fixed

Copy link
Member

@xarkes xarkes left a comment

Choose a reason for hiding this comment

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

It looks good to me, I have a few questions though, see my comments.
I did not test it on my local machine yet.


// Zoom data
qreal current_scale = 1.0;

int unscrolled_render_offset_x = 0;
int unscrolled_render_offset_y = 0;
int offset_x = 0;
Copy link
Member

Choose a reason for hiding this comment

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

I would add a comment to specify what's the meaning of offset_x and offset_y.
Because I know what your PR does, I know what the offset is for, but otherwise it can be tricky to understand imho.
I believe a \brief would be helpful here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

commenting comes to the whole graph code after this gets merged, not only here.

animation_x->setStartValue(horizontalScrollBar()->value());
animation_x->setEndValue(target_x);
animation_x->setEasingCurve(QEasingCurve::InOutQuad);
animation_x->start(QAbstractAnimation::DeleteWhenStopped);
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove this? Is it handled by centerX followed with blockTransitionedTo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because I just thought animation only slows down the users and does not mean anything. this was just copy-pasted from x64dbg code anyway and i don't think we wanna keep this.

Copy link
Member

Choose a reason for hiding this comment

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

No, the animation was added after x64dbg and does not come from their codebase.
If they have it nowadays it's probably because they took it from us.

@@ -95,15 +95,14 @@ class GraphView : public QAbstractScrollArea
~GraphView() override;
void paintEvent(QPaintEvent *event) override;

// Show a block centered. Animates to it if animated=true
void showBlock(GraphBlock &block, bool animated = false);
void showBlock(GraphBlock *block, bool animated = false);
Copy link
Member

Choose a reason for hiding this comment

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

So everything is animated now?

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, intentionally.

@ITAYC0HEN ITAYC0HEN merged commit f8cebe0 into rizinorg:master Feb 16, 2019
@ITAYC0HEN
Copy link
Member

Thank you! Very nice :D

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

Successfully merging this pull request may close these issues.

None yet

3 participants