fix #298126: added the ability to rotate and offset instrument names #5868

SKefalidis · 2020-03-25T10:40:47Z

Resolves: https://musescore.org/en/node/298126

Added the ability to rotate and offset in the TextBase class and added the relevant UI in the EditStaff menu.

I signed CLA
I made sure the code in the PR follows the coding rules
I made sure the code compiles on my machine
I made sure there are no unnecessary changes in the code
I made sure the title of the PR reflects the core meaning of the issue you are solving
I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
I made sure the commit message title starts with "fix #424242:" if there is a related issue
[] I created the test (mtest, vtest, script test) to verify the changes I made

igorkorsukov · 2020-03-25T13:06:57Z

libmscore/part.h

@@ -55,6 +55,9 @@ enum class PreferSharpFlat : char {
 class Part final : public ScoreElement {
      QString _partName;            ///< used in tracklist (mixer)
      InstrumentList _instruments;
+      int _instrumentNameRotation { 0 };
+      int _instrumentNameOffsetX { 0 };
+      int _instrumentNameOffsetY { 0 };


Offsets are best replaced with a single variable of type QPoint

igorkorsukov · 2020-03-25T13:08:55Z

libmscore/textbase.h

@@ -243,11 +243,17 @@ class TextBase : public Element {
      int  hexState                 { -1    };
      bool _primed                  { 0 };

+      QRectF _basebbox; // used to rotate and offset the text
+      int _offsetX { 0 }, _offsetY { 0 };


Offsets are best replaced with a single variable of type QPoint

That's great, I'll take care of it later today :-)

@igorkorsukov I used QPointF instead of QPoint because e.readPoint() returns a QPointF.

I would like to hear your opinion on the following matter. Should I change the arguments of setOffsets/setInstrumentNameOffset from 2 ints(x and y) to a QPointF? I like how it is very simple to just pass 2 numbers and be done with it, but I can't help but think that passing a QPointF is cleaner.

Jojo-Schmitz · 2020-03-25T15:19:07Z

mscore/editstaff.ui

+           <number>-90</number>
+          </property>
+          <property name="maximum">
+           <number>90</number>


wouldn't +/-180 make sense too?

and a tooltip about what turns clockwise and what counter clock wise

@Jojo-Schmitz

wouldn't +/-180 make sense too?

The letters would be upside down, wouldn't they?

and a tooltip about what turns clockwise and what counterclockwise

Sounds good. I should probably add tooltips for the offsets too.

yes, 180° is upside down. Not sure it is of much use, but why restrict.

Jojo-Schmitz · 2020-03-26T08:23:35Z

mscore/editstaff.ui

+        <item row="2" column="0">
+         <widget class="QLabel" name="rotationLabel">
+          <property name="toolTip">
+           <string>Negative values rotate counterclowise, positive values clockwise (-180...180).</string>


Typo "counterclowise". Also not sure whether we want to show the limits, but if, then with a "°" sign

I believe that displaying the limits will help. And you are right about the sign, I'll add units in all of them.

Jojo-Schmitz · 2020-03-26T08:23:44Z

mscore/editstaff.ui

+        <item row="2" column="1">
+         <widget class="QSpinBox" name="rotationInput">
+          <property name="toolTip">
+           <string>Negative values rotate counterclowise, positive values clockwise (-180...180).</string>


Jojo-Schmitz · 2020-03-26T08:24:25Z

mscore/editstaff.ui

+        <item row="3" column="1">
+         <widget class="QSpinBox" name="spinBoxOffsetX">
+          <property name="toolTip">
+           <string>Negative values move left, positive values move right (-200, 200).</string>


not sure about whether to show the limits, but if, then with the unit ("sp"?)

Jojo-Schmitz · 2020-03-26T08:24:36Z

mscore/editstaff.ui

+        <item row="4" column="1">
+         <widget class="QSpinBox" name="spinBoxOffsetY">
+          <property name="toolTip">
+           <string>Negative values move upwards, positive values move downwards (-200, 200).</string>


Jojo-Schmitz · 2020-03-26T08:25:21Z

mscore/editstaff.ui

+        <item row="3" column="0">
+         <widget class="QLabel" name="labelOffsetX">
+          <property name="toolTip">
+           <string>Negative values move left, positive values move right (-200, 200).</string>


not sure about whether to show the limits, but if, then with the unit ("sp"?)

Jojo-Schmitz · 2020-03-26T08:25:29Z

mscore/editstaff.ui

+        <item row="4" column="0">
+         <widget class="QLabel" name="labelOffsetY">
+          <property name="toolTip">
+           <string>Negative values move upwards, positive values move downwards (-200, 200).</string>


not sure about whether to show the limits, but if, then with the unit ("sp"?)

Jojo-Schmitz · 2020-03-26T08:26:33Z

mscore/editstaff.ui

-    <width>760</width>
-    <height>600</height>
+    <width>797</width>
+    <height>757</height>


Can't we avoid this change? Just not commit this chunk?

@Jojo-Schmitz
I was forced to increase the height (not sure when the width changed) because there wasn't enough space for the spinboxes. The thing is that as I've said in the Telegram chat, I have 2 conflicting pull requests with UI changes. No matter which one gets merged first I'll be forced to make changes in the UI.

Jojo-Schmitz · 2020-03-26T08:27:33Z

mscore/editstaff.ui

-     <x>248</x>
-     <y>254</y>
+     <x>650</x>
+     <y>677</y>


Jojo-Schmitz · 2020-03-26T08:27:39Z

mscore/editstaff.ui

-     <x>316</x>
-     <y>260</y>
+     <x>718</x>
+     <y>677</y>


elerouxx · 2020-08-17T04:43:47Z

mscore/editstaff.ui

+        <item row="3" column="1">
+         <widget class="QSpinBox" name="spinBoxOffsetX">
+          <property name="toolTip">
+           <string>Negative values move left, positive values move right (-200sp, 200sp).</string>


It will be great to have offsets, thanks for creating this PR.
Will these limits be enough for cutaway scores? Sometimes a huge horizontal indent is needed:

Jojo-Schmitz · 2020-08-17T10:29:09Z

rebase needed

RobFog · 2020-09-14T09:19:53Z

Rebase still necessary.

vpereverzev · 2020-12-31T00:05:45Z

Archived due to long inactivity. Could be re-opened later

SKefalidis force-pushed the textbase_rotate_offset branch 5 times, most recently from e846a59 to d2daf3e Compare March 25, 2020 12:10

igorkorsukov reviewed Mar 25, 2020

View reviewed changes

Jojo-Schmitz reviewed Mar 25, 2020

View reviewed changes

SKefalidis force-pushed the textbase_rotate_offset branch from d2daf3e to dcb3e1d Compare March 25, 2020 23:07

Jojo-Schmitz reviewed Mar 26, 2020

View reviewed changes

fix #298126: added the ability to rotate and offset instrument names

92293cc

SKefalidis force-pushed the textbase_rotate_offset branch from dcb3e1d to 92293cc Compare March 26, 2020 09:09

elerouxx reviewed Aug 17, 2020

View reviewed changes

vpereverzev added the archived PRs that have gone stale but could potentially be revived in the future label Dec 31, 2020

vpereverzev closed this Dec 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix #298126: added the ability to rotate and offset instrument names #5868

fix #298126: added the ability to rotate and offset instrument names #5868

SKefalidis commented Mar 25, 2020

igorkorsukov Mar 25, 2020 •

edited

igorkorsukov Mar 25, 2020

SKefalidis Mar 25, 2020

SKefalidis Mar 26, 2020 •

edited

Jojo-Schmitz Mar 25, 2020

Jojo-Schmitz Mar 25, 2020

SKefalidis Mar 25, 2020

Jojo-Schmitz Mar 25, 2020 •

edited

Jojo-Schmitz Mar 26, 2020 •

edited

SKefalidis Mar 26, 2020

Jojo-Schmitz Mar 26, 2020

Jojo-Schmitz Mar 26, 2020

Jojo-Schmitz Mar 26, 2020

Jojo-Schmitz Mar 26, 2020

Jojo-Schmitz Mar 26, 2020

Jojo-Schmitz Mar 26, 2020

SKefalidis Mar 26, 2020

Jojo-Schmitz Mar 26, 2020

SKefalidis Mar 26, 2020

Jojo-Schmitz Mar 26, 2020

SKefalidis Mar 26, 2020

elerouxx Aug 17, 2020

Jojo-Schmitz commented Aug 17, 2020

RobFog commented Sep 14, 2020

vpereverzev commented Dec 31, 2020

fix #298126: added the ability to rotate and offset instrument names #5868

fix #298126: added the ability to rotate and offset instrument names #5868

Conversation

SKefalidis commented Mar 25, 2020

igorkorsukov Mar 25, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SKefalidis Mar 26, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jojo-Schmitz Mar 25, 2020 • edited

Choose a reason for hiding this comment

Jojo-Schmitz Mar 26, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jojo-Schmitz commented Aug 17, 2020

RobFog commented Sep 14, 2020

vpereverzev commented Dec 31, 2020

igorkorsukov Mar 25, 2020 •

edited

SKefalidis Mar 26, 2020 •

edited

Jojo-Schmitz Mar 25, 2020 •

edited

Jojo-Schmitz Mar 26, 2020 •

edited