Question regarding SavePositions(...) ndpi variable
Hi !
I'm trying to figure out what the variable ndpi, defined in MuseScore::savePositions(Score*, QIODevice*,bool) is representing and why it's computed that way, (line 57 in https://github.com/musescore/MuseScore/blob/c9f8a138529c15bf9d096e53a4a…)
I asked few (in fact a lot) of people what is the meaning/intention/purpose of applying a "dpi factor" to a distance in pixel unit (line 92 to 95), and they ended up having the same interrogations as me (most working in graphic/printing industry) as it doesn't make much sense (yet) if seen as "pixel_distance * dpi", so I'm clearly missing something, does someone knows why it's done ? What does the resulting values of lines 92 to 95 unit is and "means" ?
I also browsed the file history, but it didn't helped much to figure this out,
Have a great day and thanks in advance for your answers/help, I feel like I'm missing something,
love and play.
EDIT: also, re-read the subsection description I might be in the wrong part of the forum, if so, let me know, I'll ask it in another place, just saw this as the "developer subforum", but might totally be wrong.
Comments
Did you check
git blame
to see who changed/added this when and in which commit? That may give a hint at the why.In reply to Did you check git blame to… by Jojo-Schmitz
I did, but not deep enough perhap's,
the current formula comes from "Change to new preferences model" (https://github.com/musescore/MuseScore/commit/2b6cb8b432612d19589f702d2…)
the "ndpi" factor started to be applied to pixel position with "fix pos output format; scale position to 100dpi" (https://github.com/musescore/MuseScore/commit/550bf016715e51a346142993c…)
the factor 12 comes from "more options to export positions" (https://github.com/musescore/MuseScore/commit/739cb9c99543f665b0f9fcbcd…)
But I don't really understand what is the meaning of "pixel_distance * dpi" as those are two units that don't "talk to each other" that way from my understanding.
I'll continue to dig, based on your answer, I should be able to perhap's figure it out myself looking at the changes related to those commits.
In reply to I did, but not deep enough… by Emmanuel Istace
IMHO it should be just pixel distance (breaking old .mpos/.spos files with the change), and the sx="0" bug must be fixed for this to become useful.
@devs: What does the proprietary musescore.com backend do? AIUI they also use the mpos files.
In reply to IMHO it should be just pixel… by mirabilos
It would perhap's be to consider to add another export (mxpos/sxpos, or mposx/sposx, you'll decide) that export the pixel distance, basically the same logic but not applying that dpi factor.
So it would remain compatible with any legacy client using the mpos/spos format (changing that would be a big breaking change for some "unknown" client) and help people who don't have the 'historic'.
I can handle it myself if positive feedback here, it's quite straight forward.
In reply to It would perhap's be to… by Emmanuel Istace
I’d agree if this was some kind of API that had any public consumers.
As is, this is a local export format. It’s not got a schema definition, and it even uses the same format for mpos and spos. I’d say just change it, and maybe add a version="2" to the top-level tag “score”.
In reply to I’d agree if this was some… by mirabilos
At least from this post https://musescore.org/en/node/307253 it looks like there is, relying on "non standardized" file format shouldn't be a reason to not consider them maybe ? It's the kind of features that don't create issues until they break, I'll ask opinions on the irc chat, just to know others opinion and not waste my and others time, and it's a "big responsability/decision" too as I'm really new to perhap's contrib to ms, just by precaution ^^ Also, it can be implemented without code duplication so that wouldn't mean bigger code base to maintain. (to have another pair or file format)
In reply to At least from this post… by Emmanuel Istace
Yes, but ① this change is likely to need only a single change in their user, ② it will only affect those upgrading MuseScore, and ③ the spos format is completely broken in 3.x anyway and can never have worked…
In reply to Yes, but ① this change is… by mirabilos
That's some really convincing arguments, will be done tomorrow :)
In reply to Yes, but ① this change is… by mirabilos
So, it's now done in my fork in branch 3.x, but, at last minute, didn't changed "mpos/spos" export, added "sposx/mposx", safer until I have a feedback from the person using the files "as-is" (sent a private message as well as the comment on his thread, I still hope to be able to figure out the reason), took no time, so if rejected, will take no time neither to do the other way, but I felt more "safe" with that approach, should introduce no regression/obscure legacy compat issues. Will also fix the x=0 in spos, also think there's some issues related to y, as boxing result is sometime "not precise"
TLDR; 1. Before submitting PR, should I create an issue related to this or can I link directly to this thread?
2. Also, once accepted, am I responsible for updating website documentation? Or should I contact a maintainer?
Have a great day,
love and play.
In reply to So, it's now done in my fork… by Emmanuel Istace
In reply to It's best to create an issue… by mike320
The hanbbook page for the commandline options (https://musescore.org/en/handbook/command-line-options) doesn't mention any versions though, as it is (tried to be) kept in sync with https://github.com/musescore/MuseScore/blob/master/build/Linux%2BBSD/ms… (which is what your PR should modify too)
Not 100% sure whether we should be mentioned version in the man-page or handbook though
In reply to The hanbook page for the… by Jojo-Schmitz
> Comment by Jojo-Schmitz:
>
> The hanbook page for the commandline options doesn't mention any versions
> though, as it is (tried to be) kept in sync with
> [1] https://github.com/musescore/MuseScore/blob/master/build/Linux%2BBSD/ms…
> (which is what your PR should modify too)
If you need help with the syntax of that file (mdoc manpage), ping me (also in IRC).
(Still thinking you shouldn’t duplicate the code, just write a version 2 of that file; this is how .mscx/.mscz were also handled.)
In reply to > Comment by Jojo-Schmitz: >… by mirabilos
Hey !
I didn't duplicated any code at all ! That would be a really poor practice, wathever the reason, code duplication should never be an option, would rather refactor a lot (with proper testing) than duplicate code.
I introduced a new bool argument "ndpiScaling" and ndpi is now set with:
ndpi = dpiScaling ? previousFormula : 1.
So the logic remains mostly untouched to get one or the other.
You can check my changes there: https://github.com/musescore/MuseScore/compare/3.x...Manu404:3.x
So, indeed, that would introduce breaking change with people writting code against that interface but I'm not sure anyone is doing that as it's clearly not part of any api and all tests runned successfully.
And will do the doc tonight, will join you on irc if any troubles ! Thx for the help !
Hope you're a bit reassured ^^
In reply to Hey ! I didn't duplicated… by Emmanuel Istace
You could get away with less code changes if using a default value (of
true
) for that new last argumentIn reply to You could get away with less… by Jojo-Schmitz
Thanks for the remark, will update :)
In reply to Did you check git blame to… by Jojo-Schmitz
And according to this thread... it looks like a "forgotten realm" of musescore haha : https://musescore.org/en/node/307253
#311950: Unknown units for x/y/sx/sy in spos/mpos files
Hi Emmanuel and others,
I remember we had a hard time at some point to figure out how to handle these absolute values ourselves, but it' a long time ago. I also think we asked Nicolas at some point about that...
I had a quick look in our code where we parse the .mpos file values, and found this as a developer note (we use fractional coordinates in the interval [0, 1] internally):
But I also see in the code surrounding it that we currently set a const as follows:
const double MuseScoreDPI = 360;
and I found a commit log message (from last June actually) saying this:
"adjusted assumed MuseScore export dpi for PNG files to 360 (default value for MS3; was 300 before)
--> fixes issue with incorrectly placed bounding boxes"
To be honest, I also don't know (or remember) where exactly that factor 12 came from... I hope this helps somehow (but I doubt it).
Thanks for looking into this, by the way.
If you fix that other issue with the segment positions sx attributes being mostly 0 (for the .spos files), I'd love to hear about it!