| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
MaxSize support is probably pretty slow, and will be caught out by strings with combining characters (need to do some research on that), but gives pretty reasonable results. It could be performed more quickly by render contexts with access to glyph information (e.g. SkiaSharp), where cropping the line would be logarithmic (i.e. irrelevant) rather that log-linear. WPF (the bottom-most line is drawn clipped) Skia on WPF (the bottom-most line is skipped completely) |
Sorry, something went wrong.
|
@Jonarw would you mind taking a gander at the changes in Skia, and let me know if this looks too abhorrent to you to be an acceptable change? Having done the provisional implementation of maxSize, I think putting all this stuff in one place is a good idea, but it may be inappropriate to clip by text rather than pixels for e.g. SkiaSharp if it can do clipping like occurs with WPF. That might motivate adding additional functionality to ArrangeText so that it can arrange according to maxSize but not perform the clipping itself. |
Sorry, something went wrong.
|
Generally I think the ArrangeText() function is a very good idea to make text rendering more consistent between the platforms. However I think we should think a bit more about what exactly this should do. I am a bit sceptical if clipping on character level is really the way to go for implementing maxSize. I think this could be unreliable for certain character combinations, and performance will probably fairly bad (though this should not be our main concern). I am more concerned that we might be trying to do something very complex that we don't actually really need (see also my comment in #1559) - after all, I haven't missed this feature on SkiaSharp so far. |
Sorry, something went wrong.
|
I am also not too sure how about I feel about guessing the font metrics (ascent, descent, leading) by doing multiple text measurements. At the current state, the exact algorithm for text height measurement should be an implementation detail of the render context, which we shouldn't rely on. If we effectively move the responsibility for vertical text arrangement out of the render context and into the ArrangeText() function, we should also adapt the API accordingly to make the responsibilities clear. This could mean:
|
Sorry, something went wrong.
|
@Jonarw yeah, it's pretty horrendous, and I think what you describe is actually close to how OxyPlot was designed: DrawText is even documented as not working with multi-line text. To that end, I would suggest keeping the current API as it is (i.e. MeasureText measures whole texts, DrawText should support multi-line text), but add additional methods which provide what you describe (i.e. measure width of text, retrieve metrics for a given font/size/weight combination). It could be that these are not made available through IRenderContext - because axes and series and what-not shouldn't need them - but through another ITextMeasurer interface. That would avoid any breaking changes, and make it easy to push all the text-arranging stuff into it's own class. (This new class could provide methods for computing the multi-text widths/heights, so all the render context has to do is implement the basic features you describe). |
Sorry, something went wrong.
|
You are right of course, from the perspective of Axes, Series etc. the IRenderContext interface should not change. I haven't completely wrapped my mind around this, I need to think about it some more... |
Sorry, something went wrong.
|
Thanks for the comments on what must look a complete mess at the moment. I'll probably next implement your suggestions about abstracting out the 'basic' measurement stuff (so we don't have to infer metrics), and put the whole thing in its own class. Rough plan:
The additional interface will duly provide the information the managed only SvgRenderContext needs to do probably text aligning because it won't have to guess the descent anymore. |
Sorry, something went wrong.
|
Recent changes:
Next tasks:
Any feedback (esspecially negative) welcome as always. |
Sorry, something went wrong.
|
Refitting done, so ImageRenderContext, SvgRenderContext, and SkiaRenderContext shouldn't change any more. @Jonarw all else aside, does the interface to TextArranger look half-sane? Also refitted the TextTrimmer so it supports Unicode and adding additional tests. This of course has made it slower... but I don't think that should be a concern: it is rarely used, and would be providing a capability that is presently unavailable almost everywhere. I doubt it is perfect (i.e. there will be strings that it gives nonsense results on), but it should be fairly easy to extend by making the central regexes more complicated. Annoyingly we can't compile the regexes because we started .NET Standard 1.0; however, the architecture means that if someone really cared they can swap out the text trimmer for their own; whether this is a good architecture is another issue. WinForms is still broken: either I'm having difficultly extracting the font metrics (i.e. Graphics.RenderContext.GetFontMetrics is broken), or DrawString is doing something unexpected. |
Sorry, something went wrong.
|
Fixed WinForms. I think that means everything is working as I understand it should. I've not StyleCop'd this, but I'm going to open for review because there is nothing else I'm intending to do at the moment. |
Sorry, something went wrong.
|
Looks very promising! Just for my understanding: This adapts ImageSharp, SkiaSharp, WinForms and Pdf, at the moment, right? So WPF would still have to be adjusted for this (but could also be its own issue and PR). Also we should then add more sophisticated ITextTrimmers where possible (i.e. for SkiaSharp) - but again, this does not have to be part of this PR in my opinion. |
Sorry, something went wrong.
|
@Jonarw thanks. I'll eagerly await your feedback, and perhaps put my efforts into #1516 It also adapts the built-in SVG renderer, which now depends on an ITextMeasurer rather than a whole IRenderTarget, which is something that could be exploited in the future. I've not refitted WPF yet because frankly I'm frightened of it, but I can do that work for this PR or it can be done later if @objorke approves. For the purposes of consistent vertical alignment it is probably a good idea, but it may still be that we leave the trimming to it (though as I've discussed elsewhere, trimming by character seems a much better strategy to me) which could complicate matters with multi-line text Regards advanced trimmers: yep, that was pretty much what I had in mind. The performance so far is perfectly good in my opinion, but there is plenty of scope for improvement. I won't claim the current trimmer behaviour is perfect, but it should be fairly easy to adapt if we find difficulties, and the architecture means it should be easy to support users without regular releases. |
Sorry, something went wrong.
|
Rebase against develop. |
Sorry, something went wrong.
|
Should idealy review https://www.unicode.org/reports/tr14/tr14-40.html and http://www.unicode.org/reports/tr14/tr14-22.html to try to conform to unicode standards, but I don't think this is essential for this PR given there was no support for anything prior and it will probably be very difficult to write something standard complaint (if anyone knows better, please do speak up), so we should take our time over it. |
Sorry, something went wrong.
|
Not sure why the test is failing |
Sorry, something went wrong.
|
Failing on the XKCD examples, something to do with font. |
Sorry, something went wrong.
|
New code is crashing because it insists on measuring the text when rendering, which the old code didn't. Not immediately sure how to deal with this. Should we be throwing if the font isn't available, or should the drawing routines trap the exception and use a default? Certainly need to be able to detect when the font is missing so we can use a consistent default if we go that way. Example stack trace: Failed ExportPngAndCompareWithBaseline(Test #1) [53 ms]
Error Message:
System.InvalidOperationException : No glyph typeface found
Stack Trace:
at OxyPlot.Wpf.CanvasRenderContext.GetFontMetrics(String fontFamily, Double fontSize, Double fontWeight) in /_/Source/OxyPlot.Wpf/CanvasRenderContext.cs:line 463
at OxyPlot.Rendering.TextArranger.MeasureText(String text, String fontFamily, Double fontSize, Double fontWeight) in /_/Source/OxyPlot/Rendering/TextArranger.cs:line 197
at OxyPlot.Rendering.TextArranger.ArrangeText(ScreenPoint p, String text, String fontFamily, Double fontSize, Double fontWeight, Double rotation, HorizontalAlignment horizontalAlignment, VerticalAlignment verticalAlignment, Nullable`1 maxSize, HorizontalAlignment targetHorizontalAlignment, TextVerticalAlignment targetVerticalAlignment, String[]& lines, ScreenPoint[]& linePositions) in /_/Source/OxyPlot/Rendering/TextArranger.cs:line 78
at OxyPlot.Wpf.CanvasRenderContext.DrawText(ScreenPoint p, String text, OxyColor fill, String fontFamily, Double fontSize, Double fontWeight, Double rotation, HorizontalAlignment horizontalAlignment, VerticalAlignment verticalAlignment, Nullable`1 maxSize) in /_/Source/OxyPlot.Wpf/CanvasRenderContext.cs:line 344
at OxyPlot.XkcdRenderingDecorator.DrawText(ScreenPoint p, String text, OxyColor fill, String fontFamily, Double fontSize, Double fontWeight, Double rotate, HorizontalAlignment halign, VerticalAlignment valign, Nullable`1 maxSize) in /_/Source/OxyPlot/Rendering/RenderContext/XkcdRenderingDecorator.cs:line 133
at OxyPlot.PlotModel.RenderErrorMessage(IRenderContext rc, String title, String errorMessage, Double fontSize) in /_/Source/OxyPlot/PlotModel/PlotModel.Rendering.cs:line 155
at OxyPlot.PlotModel.RenderOverride(IRenderContext rc, OxyRect rect) in /_/Source/OxyPlot/PlotModel/PlotModel.Rendering.cs:line 134
at OxyPlot.PlotModel.OxyPlot.IPlotModel.Render(IRenderContext rc, OxyRect rect) in /_/Source/OxyPlot/PlotModel/PlotModel.Rendering.cs:line 34
at OxyPlot.Wpf.PngExporter.ExportToBitmap(IPlotModel model) in /_/Source/OxyPlot.Wpf/PngExporter.cs:line 99
at OxyPlot.Wpf.PngExporter.Export(IPlotModel model, Stream stream) in /_/Source/OxyPlot.Wpf/PngExporter.cs:line 73
at OxyPlot.Wpf.PngExporter.Export(IPlotModel model, String fileName, Int32 width, Int32 height, Double resolution) in /_/Source/OxyPlot.Wpf/PngExporter.cs:line 62
at OxyPlot.Wpf.Tests.ExampleLibraryTests.<ExportPngAndCompareWithBaseline>g__ExportAndCompareToBaseline|4_0(PlotModel model, String fileName, <>c__DisplayClass4_0& ) in /_/Source/OxyPlot.Wpf.Tests/ExampleLibraryTests.cs:line 52
at OxyPlot.Wpf.Tests.ExampleLibraryTests.ExportPngAndCompareWithBaseline(ExampleInfo example) in /_/Source/OxyPlot.Wpf.Tests/ExampleLibraryTests.cs:line 64
|
Sorry, something went wrong.
|
Text layout always involves measuring so that line breaks can be handled. |
Sorry, something went wrong.
|
Aye, I think it can't be avoided, but we need to decide how to handle a missing font when rendering: previously it would just be passed through to WPF and it would default to a different font. My preference at the moment would probably be to implement that behaviour explicitly, so we handle falling back to a different font, maybe allowing the user to configure if they want it to throw in this case or not. |
Sorry, something went wrong.
|
Pushed a change so that CanvasRenderContext uses the default font if it can't get font metrics for the time being. |
Sorry, something went wrong.
|
Rebased. @Jonarw do you have any opinion on the issue of default fonts? (see recent comments above and most recent commit) |
Sorry, something went wrong.
|
I think falling back to the default font seems sensible. How do other renderers handle this? As there are no crashes in the examples I assume they also just fall back to the default? |
Sorry, something went wrong.
|
Rebased against master: @Jonarw given how slowly things are moving at the moment, I'd suggest we (squash) and merge this, and give ourselves time to find any remaining issues with the understanding that this isn't necessary a completely fixed API yet. If this massive PR gets out of the way, I might get up the courage to start re-writing the Axis abstractions per #1640. |
Sorry, something went wrong.
There was a problem hiding this comment.
I am sorry for my prolonged absence, this came during a quite busy time for me, and then I just forgot about it.
I agree with you and would be happy to have this merged! I just found one property that I think is a probably a remnant of an earlier version of CanvasRenderContext?
Sorry, something went wrong.
| /// </summary> | ||
| /// <value><c>true</c> if stream geometry should be used; otherwise, <c>false</c> .</value> | ||
| /// <remarks>The XamlWriter does not serialize StreamGeometry, so set this to <c>false</c> if you want to export to XAML. Using stream geometry seems to be slightly faster than using path geometry.</remarks> | ||
| public bool UseStreamGeometry { get; set; } |
There was a problem hiding this comment.
I don't think this is used anywhere?
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fixes #1531, #1559, and #1554
Checklist
Changes proposed in this pull request:
Big Issues
@oxyplot/admins