Add contour draw option to H.Metrics. #5496
Conversation
vasilky3
force-pushed
the
feature/add-contours-to-fontview
branch
from
7d7731d to
fdcbda7
Compare
fix cp to Cmake copy
vasilky3
force-pushed
the
feature/add-contours-to-fontview
branch
from
fdcbda7 to
3a1679d
Compare
…y() in order to stay DRY
|
@skef, please take a look. |
| int height = 0; | ||
| for (spl = set; spl != NULL; spl = spl->next) | ||
| { | ||
| Color fc = spl->is_clip_path ? clippathcol : fg; |
There was a problem hiding this comment.
I haven't tried this, but from the code it seems like you turn on the metrics option and you get the outline in the foreground color? Is that always going to make sense? What if you have a mix of closed and open contours (maybe you're in the middle of developing a stroked font) and want to see the contrast?
There was a problem hiding this comment.
I guess from the attached image it's already a different color ...
There was a problem hiding this comment.
The image is corresponds to this code.
fg is passed as function argument.
In fontview.c it's called with green color:
CVDrawSplinePointList(NULL, pixmap, sc->layers[1].splines, fvmetadvancetocol, sfm_stroke, x0, y0, (double) (box.width - 1) / sc->vwidth);
There was a problem hiding this comment.
Hmm, so we're reusing a metrics color: "Color used to draw the (horizontal or) vertical line from the origin to the advance when that metric is selected in View". That's not ideal. Could we maybe add a color to fontview_re?
There was a problem hiding this comment.
(I admit this is hairy, as it will also involve changes to the appearance editor ...)
There was a problem hiding this comment.
As far as I can see, there is no need to mess with Appearance Editor specifically, the new color definition should show up there automagically.
There was a problem hiding this comment.
@iorsh thx for info. Helped me alot.
there is no need to mess with Appearance Editor specifically
it's true.
There was a problem hiding this comment.
There was a problem hiding this comment.
One more quick thing: Make an entry for this using the case-sensitive string you added in fontview_re ("MetricsGlyphContourColor") around here:
. ("Tango" is the current resource directory, I don't think it's necessary to mess with 2012 as well but you're free to.) For the PR you can use 0x3030c0 like you did as the internal program default as long as that makes sense with the other colors, as in:fontforge.FontView.MetricsGlyphContourColor: #3030C0
but I would also advise changing it and restarting the program to ensure the value is getting picked up properly.
@vasilky3 from your original screenshots it looks like you maybe didn't the resource directory configured for your system, but the more recent screenshot of the appearance editor looks more like what I would expect. Did you change something/get that working?
There was a problem hiding this comment.
Done. Here is the result.
Previously i was trying to build fontforge from this repo by my own.
This time I did read wiki.
Now I'm using FontForgeBuild repo for building.
Got some problems with understanding how to force it to build my branch. But got it finally.
I'm working with cnc fonts. And it's hard to navigate dew to lack of glyph splines visualisation in the MainWindow.
So i added one.
Type of change
Code quality
I would say it's low. But somewhat consistent with surrounded code.
My main problem new method
fontview.c :: FVDrawOutlineOnly()probably should be placed ingdrawP.h :: displayfuncs.But i was confused trying to figure out how to achieve this.
Sorry for not putting spacing refactor in a separate commit.