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

Fix #84416 : Allow long note groups to scroll or pan in Create Time Signature dialog #2288

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@AntonioBL
Copy link
Contributor

AntonioBL commented Nov 12, 2015

No description provided.

@AntonioBL AntonioBL force-pushed the AntonioBL:exampleviewscroll branch from 8cdebe1 to f914b1e Nov 12, 2015

@IsaacWeiss

This comment has been minimized.

Copy link
Contributor

IsaacWeiss commented Jul 8, 2016

@lasconic: @duck57 and JGitar requested this again at https://musescore.org/en/user/101731/blog/2016/05/01/musescore-3.0-under-development-musescore-gets-smart#comment-527681, and the PR is still conflict-free and ready to merge eight months later.

@lasconic

This comment has been minimized.

Copy link
Member

lasconic commented Jul 8, 2016

@IsaacWeiss Meaning you tried it?

lastPage->pos().y() * _matrix.m11(),
lastPage->width() * _matrix.m11(),
lastPage->height() * _matrix.m11());
QRectF pagesRect = firstPageRect.unite(lastPageRect).translated(offsetPt);

This comment has been minimized.

@lasconic

lasconic Jul 8, 2016

Member

unite is deprecated in Qt >4.8. We can use united.

@IsaacWeiss

This comment has been minimized.

Copy link
Contributor

IsaacWeiss commented Jul 8, 2016

Actually, no. I would have guessed somebody else would have, and
reported a problem before now if there was one. Compilation takes a long
time on my computer, but I'll give it a go.

On 7/8/16 10:34 AM, Nicolas Froment wrote:

@IsaacWeiss https://github.com/IsaacWeiss Meaning you tried it?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2288 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ALE_mYiV-FlPRRaSQveJFPBjGWBpDpcSks5qTl_5gaJpZM4GhD-H.

@lasconic

This comment has been minimized.

Copy link
Member

lasconic commented Jul 8, 2016

Replacing unite by united, I compiled this PR. And I can't see the measure entirely.

capture d ecran 2016-07-08 16 36 17

@AntonioBL

This comment has been minimized.

Copy link
Contributor Author

AntonioBL commented Jul 8, 2016

In the PR I was simply copying the part of code from scoreview.cpp responsible for the panning of the score. Maybe with the recent changes it must be tweaked a little (I think the vertical shift is due to these changes; a comparison with scoreview.cpp should highlight the problem).
If I remember correctly, one of the defects I found in the implementation I did here is that if you pan completely to one side and then come back, it does not go to the initial position, but it is shifted by a few pixels.
If I have time in the weekend I can try to update the PR.

@lasconic

This comment has been minimized.

Copy link
Member

lasconic commented Jul 8, 2016

@abl great. Thanks!

@AntonioBL

This comment has been minimized.

Copy link
Contributor Author

AntonioBL commented Jul 13, 2016

Sorry that I am late in answering, but I was sick this weekend.
First, a note: the exampleview shown by Nicolas looks like in that picture even without this PR. Indeed, this PR only adds the horizontal scrolling functionality to the exampleview. However, the boundary limits are no more correct with the new changes. I am trying to identify the new values needed in this case; hopefully, I will also find where the vertical offset is set and correct it.
It will take some time (unfortunately I have little free time available).

@AntonioBL AntonioBL force-pushed the AntonioBL:exampleviewscroll branch from f914b1e to 470afd8 Jul 14, 2016

@AntonioBL

This comment has been minimized.

Copy link
Contributor Author

AntonioBL commented Jul 14, 2016

I tried to tackle the problems that were present.
I don't know if it works correctly for high DPI display (and GUI scaling). How can I check this?

EDIT:
It seems to work also with the "-x" option for GUI scaling, but I couldn't test o a true high DPI display.

@AntonioBL AntonioBL force-pushed the AntonioBL:exampleviewscroll branch from 470afd8 to 27f7182 Jul 28, 2016

@AntonioBL

This comment has been minimized.

Copy link
Contributor Author

AntonioBL commented Jul 28, 2016

Rebased to fix merge conflicts

@AntonioBL AntonioBL force-pushed the AntonioBL:exampleviewscroll branch from 27f7182 to e3093e7 Jan 9, 2017

@AntonioBL

This comment has been minimized.

Copy link
Contributor Author

AntonioBL commented Jan 9, 2017

And rebased again to fix merge conflicts.

@ericfont

This comment has been minimized.

Copy link
Contributor

ericfont commented Feb 18, 2017

I just tested this, rebased to latest master, and I find that the horizontal scrolling works great, except the calculation of the leftmost & rightmost limit needs to be properly calculated based on the width of the ExampleScore's current width, especially when resizing the window. Other than that, I support this PR.

I also would like if could scroll via Shift->MiddleMouseScrollWheel, just like I can do with regular score, if my mouse cursor is within the example score rectangle.

@ericfont

This comment has been minimized.

Copy link
Contributor

ericfont commented Feb 18, 2017

basically my rules would be:

  1. When moving right, ensure the left edge of system won't be right of frame's left edge margin.
  2. Never move left if entire system already fits entirely within the frame.
  3. When moving left, ensure the right edge of system won't be left of frame's right edge margin.
@ericfont

This comment has been minimized.

Copy link
Contributor

ericfont commented Feb 19, 2017

As I said, I found AntonioBL's PR allowed way too much scrolling and didn't behave nicely when the widget is resized. So I added a commit to AntonioBL's PR to obey my stricter rules and submitted this as a seperate PR #3007.

lasconic added a commit that referenced this pull request Feb 19, 2017

Merge pull request #3007 from ericfont/2288
modified PR #2288 scrolling with stricter rules and simpler code
@lasconic

This comment has been minimized.

Copy link
Member

lasconic commented Feb 19, 2017

See PR #3007.

@lasconic lasconic closed this Feb 19, 2017

@AntonioBL AntonioBL deleted the AntonioBL:exampleviewscroll branch Nov 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.