grame-cncm / guidolib

Guido project - music score layout engine - music description language
http://guido.grame.fr
Mozilla Public License 2.0
156 stars 35 forks source link

Support Circle enclosure for Mark tag #115

Closed arshiacont closed 4 years ago

arshiacont commented 4 years ago

Hello, The Mark tag does not seem to support a circle enclosure... . Circle enclosure is common for example for indicating chords in Guitar and are a good way to distinguish with fingerings. If it's easy to add, it would be great.

dfober commented 4 years ago

I'll have a look at that. As far as I remember, that was not supported because the graphic devices don't provide any function to draw a circle (there hasn't been any need for it until now.) The main problem is that all the graphic have to be updated and tested (and there's a bunch of them). Thus it's not a five minute task. As soon as I'll find the time...

dfober commented 4 years ago

That's done. Between environments, platforms and utilities, there are no less than 13 devices to update. There's still a few more to do but you can already test on your side. Note that I didn't test the OSX device (I don't have any application that uses it), nor the windows devices (GDI and GDIPlus). I strongly hope that there will be contributions if there are some problems with these implementations.

arshiacont commented 4 years ago

Thank you @dfober Count on me for OSX/iOS at first, followed by web.

arshiacont commented 4 years ago

@dfober First test on Qt: The circle does not seem to adapt if a fontSize is set.


{[ \staff<1>  \barFormat<style= "system", range="1"> 
   (* meas. 1 *)  \clef<"g2"> \key<1> \meter<"6/4", autoBarlines="off", autoMeasuresNum="system"> 
 \stemsUp  d3/4
 \stemsUp  b2/4 \mark<"2", enclosure="circle", fsize=7.5pt, dy=-17.4hs> g/4 
\mark<"3", enclosure="circle", fsize=7.5pt, dy=-17hs> e/4 
\mark<"2", enclosure="circle", fsize=7.5pt, dy=-17hs>  f#/4 g/4 \bar 
   (* meas. 2 *)  b/2. \mark<"2", enclosure="circle", fsize=7.5pt> 
\mark<"4", enclosure="circle", fsize=7.5pt, dy=-17hs> {\acc( g#1/2.), e2/2.  } ]
}

image

arshiacont commented 4 years ago

@dfober It is actually fine and not a text size adaptation problem... .

You just need to add the insets to the width instead of deducing it in on GRMark.cpp line 92 and we will be fine on Qt.

Testing OSX... .

arshiacont commented 4 years ago

Dominique,

Tested on Qt VGDevice and OSX Devices.

The GRMARK::toCircle should be changed to:

void GRMark::toCircle (const FloatRect& r, VGDevice & hdc) const
{
    float w = std::min(r.Width(), r.Height());
    float inset = 15;
    hdc.FrameEllipse(r.left + w/2, r.top + w/2, w+inset, w+inset);
}

and the GDeviceOSX::FrameEllipse to:

void GDeviceOSX::FrameEllipse( float x, float y, float width, float height)
{
    CGRect rect;
    rect.origin.x = x - width;
    rect.origin.y = y - height;
    rect.size.width = 2*width;
    rect.size.height = 2*height;
    ::CGContextAddEllipseInRect(mContext, rect);
    ::CGContextStrokePath(mContext);
}

and we're set for these platforms.

Note that the OSX device coordinate transform is 100% the same as the Qt implementation!

Since this is small change, I let you add it if satisfied! :)

dfober commented 4 years ago

ok for me, apart from that the inset affects all the devices implementations and thus, should be rather set on GDeviceOSX only.

dfober commented 4 years ago

sorry, it's already set on GRMark...

dfober commented 4 years ago

done and pushed! Tell me if it's ok and if we can close this issue.

arshiacont commented 4 years ago

All good! You might want to also add empty definitions in canvasdevice.cpp and canvasdevice.h of the javascript stuff (just did myself and it works!).

We will then be good to close.

PS: I don't see any use of "Ellipse" in GRs yet but probably the OSX implementation should be adapted when we should use them.

dfober commented 4 years ago

Just one device more... they're hiding in every corner :-) That's also done.

PS: I don't see any use of "Ellipse" in GRs yet but probably the OSX implementation should be adapted when we should use them.

Strangely no, so far there hasn't been any need for an ellipse.

arshiacont commented 4 years ago

Thanks!