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

279877 continuousview #5255

Closed

Conversation

rajansaini691
Copy link

Improved background layout of continuous view and fixed issue 279877

 - Made code for finding height of panel more readable
 - Extended vertical padding around the panel for aesthetic reasons
 - Should stop text from going past the panel
@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Aug 4, 2019

Please squash the commits and add "fix #279877" to the commit title

else {
painter.setMatrixEnabled(false);
painter.setMatrixEnabled(false); // Paints background when custom backing is used
painter.drawTiledPixmap(bg, *fgPixmap, bg.topLeft()
- QPoint(lrint(_sv->matrix().dx()), lrint(_sv->matrix().dy())));
painter.setMatrixEnabled(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of all these changes I found it is enough to remove the lines 284/288 and 287/291 to solve issue 279877.

@@ -28,6 +28,7 @@
#include "preferences.h"
#include "scoreview.h"
#include "continuouspanel.h"
#include <QDebug>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not be needed, as it is in all.h

for (int i = 0; i < _score->nstaves(); ++i) {
qreal _padding = 8 * _spatium;
_y = system->staffYpage(0) + system->page()->pos().y() - _padding;
double _staff_height = 0.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

whitespace damage

QList<StaffName>& staffNamesLong = currentStaff->part()->instrument(Fraction::fromTicks(tick))->longNames();
QString staffName = staffNamesLong.isEmpty() ? " " : staffNamesLong[0].name();
QString staffName = staffNamesLong.isEmpty() ? "" : staffNamesLong[0].name();
Copy link
Contributor

Choose a reason for hiding this comment

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

an empty string is different from one containing a space, is this really intended here?

@@ -1 +1 @@
3543170
19679cf
Copy link
Contributor

Choose a reason for hiding this comment

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

This change needs to go, revision.h should never ever be made part of a commit

QPixmap* fgPixmap = _sv->fgPixmap();
if (fgPixmap == 0 || fgPixmap->isNull())
if (fgPixmap == 0 || fgPixmap->isNull()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

braces not needed

@anatoly-os
Copy link
Contributor

@rajansaini691 please reopen when you are around to address the code review.

@anatoly-os anatoly-os closed this Dec 16, 2019
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.

4 participants