Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSharpMath 0.1.0-pre1, 0.1.0-pre2, 0.1.0-pre3 and 0.1.0-pre4 have been released! #12

Closed
2 of 13 tasks
Happypig375 opened this issue Jun 28, 2018 · 42 comments
Closed
2 of 13 tasks
Assignees
Labels
Resolution/Superceded The described announcement or pull request has been superceded. Type/Announcement

Comments

@Happypig375
Copy link
Collaborator

Happypig375 commented Jun 28, 2018

0.1.0-pre1 Announcement:

With the 0.1.0 SkiaSharp update, beautiful math rendering in SkiaSharp and Xamarin.Forms is now possible.
Currently in prerelease state, it is available for download for testing. Test all the packages as hard as you can! You can contribute to bug-smashing.
The master branch has been rebased onto SkiaSharp, which is planned to be deleted.

Please report any issues with this prerelease inside this issue!

Known issues

  • 1 \\ { 2 \\ 3 }'s spacing between 1 and 2 is weird
    image
  • Too few tags for the packages (suggested: formula tex render)
  • CSharpMath.Text (Text and maths #6) is not yet implemented
  • No way to limit scrolling to horizontal only or vertical only
  • CSharpMath.Forms.Example.UWP is not buildable with .NET Native Toolchain enabled
  • Loading times of CSharpMath.Forms.Android.Example is still very slow in Release mode
  • Instructions for cloning with submodules is incomplete. Suggest adding a custom analyzer. (Note: the Roslyn Analyzer API was not deemed suitable, so a conditional file with #errors was used instead.)
  • Pinching a FormsMathView has been removed due to SkiaSharp API limitations
  • No Changelog for now (will be written in some future prerelease)
  • Documentation is unfinished on both NuGet and GitHub wiki
  • Unit tests are missing in general
  • Release procedure is not documented
    And finally,
  • Promotion of CSharpMath not yet done

#1, #2, #3 and #10 will be merged into here.

Note: Active development will cease until 10th July due to travelling to mainland China. Hope to see you then!

@Happypig375
Copy link
Collaborator Author

Before After
Before After

Much better with the icon 😃

@Happypig375 Happypig375 self-assigned this Jun 28, 2018
@charlesroddie
Copy link
Collaborator

charlesroddie commented Jun 29, 2018

'Could not load file or assembly 'Typography.OpenFont, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.'

After installing CSharpMath.SkiaSharp 0.1.0-pre1, and trying this test code:

type MathView(tex) =
    inherit SkiaSharp.Views.Forms.SKGLView()
    let mathpainter = CSharpMath.SkiaSharp.SkiaMathPainter( 200.f, 100.f, AntiAlias = true, LaTeX = tex )
    override t.OnPaintSurface(e : SkiaSharp.Views.Forms.SKPaintGLSurfaceEventArgs) = mathpainter.Draw(e.Surface.Canvas)

@charlesroddie
Copy link
Collaborator

charlesroddie commented Jun 29, 2018

Also SkiaMathPainter takes an SKSize (width and height) and an optional fontsize. I don't fully understand that. What role do width and height play? Are they for resizing? Surely based on a fontsize, the resulting SKSize is given?

@Happypig375
Copy link
Collaborator Author

@charlesroddie

  • It seems to be loading a Typography DLL... I did not encounter this exception while testing the examples. The workaround is building from source right now.

  • The role for width and height during construction of SkiaMathPainter is to calculate the appropriate alignments and padding for drawing. Aside from providing in the constructor, the bounds of drawing should also be set to the Bounds property before drawing; a better way would have been required parameters for the Draw method.

@charlesroddie
Copy link
Collaborator

API Comments

Sizing

A SkiaMathPainter should require a fontsize (which can have a default if not given) and a source to construct. It shouldn't require any other measurements.

An instance of SkiaMathPainter should have a Measure:unit -> SKSize property. It should also have a Draw:SKCanvas*SKPoint->unit property. The SKPoint is the upper left coordinate, just like canvas.DrawText.

(Alternatively, a SkiaMathPainter doesn't require a font size, and should have Measure(fontsize) and Draw(canvas, point, fontsize) properties.)

You can easily make margins and centering as needed out of Measure and Draw. The same SkiaMathPainter can draw at different places, as the canvas changes size, and into diferent canvases if needed.

Sources

There are a lot of overloads in the SkiaMathPainter and MathSource w.r.t. sources, and invalid combinations of properties result.

For example you can specify a MathSource based on an IMathList and then get it's LaTeX, and I think that will fail as the LaTeX is not defined. I believe your idea for Source is to bypass LaTeX - a good idea in theory - but then there should not be LaTeX properties and definitly not properties with getters.

Suggestion: remove the potentially invalid getters, and also remove the indrect overloads.

A SkiaMathPainter depends most directly on something. Maybe it's an Atom. Let the SkiaMathPainter require that input as its source, and remove the other overloads. We could end up with:

let atom = Atom.FromTeX(tex)
let painter = SkiaMathPainter(Atom = atom, FontSize = 12.)

We could then have more constructors for Atom, e.g. Atom.FromMathML(...) if someone wants to implement that in future. Or we can define atoms directly to bypass parsing. This example (different string types) suggests different methods rather than different overloads here.

The SkiaMathPainter constructor then does not need a TeX overload getter or setter.

Actually I think the Atom here should be required in the SkiaMathPainter constructor and not settable. Since any change is effectively recreating the object anyway, there are no effiency gains in letting this property be settable.

(I am using Atom here as I don't yet know what a Source is or how it's different from an Atom.)

@Happypig375
Copy link
Collaborator Author

@charlesroddie
Removing the width and height constructor arguments, the Bounds property, and replacing them with the availableWidth and availableHeight parameters for the Draw method is planned for pre2.

I am not requiring the point of drawing to be supplied directly because in practical usage, the math painter is typically used for views. (Like your MathView that is derived from SKGLView). Ideally, the available size parameters should be directly fed to from the platform-given arguments on draw. (Now, different canvases can be drawn onto by the same math painter.) If the drawing point was to be supplied directly, the alignments of the content would need to be calculated externally, which won't be useful. Maybe I can even use the LocalClipBounds property of SKCanvas directly instead of these parameters.

Regarding the Measure property, it is provided currently by the name of DrawSize.

Getting LaTeX from a MathSource constructed with an IMathList is valid. Try it yourself.

Oh yes,

  • Support raisebox atom to LaTeX (forgot to add this)

The intention of having a MathSource is to support translations between different math languages. For each language, only two translators are needed: language->MathList and MathList->language. This way, we can get convert e.g. MathML to LaTeX without the use of an actual painter.

  • add above to documentation

All of the content properties (e.g. LaTeX) of math painters are just convenience properties for the MathSource.

If you do want to avoid the MathSource structure, use CSharpMath.Atoms.MathLists.BuildResultFromString. An Atom represents a LaTeX command, and a MathList represents a container of LaTeX commands. A MathList is inherently not an Atom as noted in the XML Documentation of IMathList. As LaTeX usually contain more than one atom, a MathList is used in here. A MathSource represents not just any one atom or any series of atoms, but the document that contains ALL the commands.

  • A new wiki page for above

A math painter's content is mutable because there is no need to reassign every other property just for a change in its content.

@charlesroddie
Copy link
Collaborator

charlesroddie commented Jul 4, 2018

If the drawing point was to be supplied directly, the alignments of the content would need to be calculated externally.

There is no need to align content in the main draw method, and that is why specifying available heights and widths should not be in there, but specifying a point should. As discussed above, it would be standard to specify the top left point. You can take the SkiaSharp drawing methods as a guide to a good API.

You are welcome to add a convenience method DrawAligned(rect:SKRect, horizontalAlignment, verticalAlignment), or overload Draw with this, but it should not be the main Draw method.

The intention of having a MathSource is to support translations between different math languages. For each language, only two translators are needed: language->MathList and MathList->language. This way, we can get convert e.g. MathML to LaTeX without the use of an actual painter.

That sounds fine. From what you are saying there is no difference in data between a MathSource and a MathList. So I would not have two separate names for these.

@Happypig375
Copy link
Collaborator Author

Happypig375 commented Jul 13, 2018

0.1.0-pre2 would have been released if not the two-factor authentication blocking myself from signing into NuGet (waiting for support).
To anyone viewing this, here are the packages to try out:
https://github.com/verybadcat/CSharpMath/releases/tag/v0.1.0-pre2

@charlesroddie
Copy link
Collaborator

Great. Did you work out how to fix the missing typography dlls?

@Happypig375
Copy link
Collaborator Author

I suspect that I just uploaded a bad .nupkg that was packed when I was trying to directly reference the builder projects from Typography instead of the normal method of referencing the shared projects.

Importing the free packages via local feed raised no exceptions, so I think the problem is solved. Can you please verify?

@Happypig375
Copy link
Collaborator Author

Happypig375 commented Jul 14, 2018

0.1.0-pre2 is now out for download on NuGet.

Major changes since 0.1.0-pre1:

  • Reworked Draw methods
  • Removed bounds constructor parameters and the Bounds property
  • Renamed LayoutBounds property to Measure
  • Should-be-fixed exception of Typography DLL not being found (please verify)
  • Plus change of the IPainter interface

@Happypig375 Happypig375 changed the title CSharpMath 0.1.0-pre1 has been released! CSharpMath 0.1.0-pre1 and 0.1.0-pre2 has been released! Jul 14, 2018
@charlesroddie
Copy link
Collaborator

charlesroddie commented Jul 14, 2018

Yes the Typography DLL issue is fixed in 0.1.0-pre2.

Thanks for redoing the api. Looks better now.
A SkiaMathPainter's Measure property is a Nullable<System.Drawing.RectangleF>. Surely you should always be able to get a measure, so it shouldn't be nullable. Also RectangleF has left,right,top,bottom. Isn't a measure height and width? SKSize and SKPoint would work for Measure and Draw.

With the code:

type MathView(tex) =
    inherit SkiaSharp.Views.Forms.SKGLView(HeightRequest = 200.)
    let mathpainter = CSharpMath.SkiaSharp.SkiaMathPainter( LaTeX = tex )
    override t.OnPaintSurface(e : SkiaSharp.Views.Forms.SKPaintGLSurfaceEventArgs) =
        mathpainter.Draw(e.Surface.Canvas, System.Drawing.PointF(0.f,0.f))

I get an unhandled exception with no debug information. Without System.Drawing.PointF(0.f,0.f) it renders OK.

@Happypig375
Copy link
Collaborator Author

Happypig375 commented Jul 14, 2018

  • Add following to documentation

The Measure property returns:

  • null if MathList is null or LaTeX contains an error (no empty sizes so that potential bugs can be avoided)
  • a rectangle with x coordinate zero, y coordinate the descent of the laid out content (so that you can know how much the starting point is below the baseline), width and height same as width and height of content.
    (Therefore, you are meant to look at Y, width and height instead of left, right, top and bottom.)

The Draw(SKCanvas, PointF) overload always throws NotImplementedException that was used to test private vs public method resolution. I forgot to change it back to using an actual implementation and will do it in the next prerelease. Use the Draw(SKCanvas, float, float) overload that takes x and y coordinates as separate parameters for now.

@charlesroddie
Copy link
Collaborator

charlesroddie commented Jul 14, 2018

There is a bug in the y coordinate. Draw(canvas,x,y) draws starting from (x,y+canvasHeight) instead of (x,y) (in SkiaSharp coordinates). So Draw(canvas,0.f,0.f) displays at the bottom left of the canvas, when it should be the top left.

Otherwise the SkiaMathPainter method is consistent with standard Skia text rendering. (I was wrong about standard text coordinates before. The point specified is at the bottom left not the top left of the text. That's OK and unrelated to the coordinate problem above.)

@charlesroddie
Copy link
Collaborator

(Deleted a comment - mistake while testing.)

@Happypig375
Copy link
Collaborator Author

@charlesroddie Is the deletion a mistake or the posting of comment a mistake? I can help repost it from email.

@charlesroddie
Copy link
Collaborator

The comment was a mistake!

@charlesroddie
Copy link
Collaborator

charlesroddie commented Jul 15, 2018

a rectangle with x coordinate zero, y coordinate the descent of the laid out content (so that you can know how much the starting point is below the baseline), width and height same as width and height of content. Therefore, you are meant to look at Y, width and height instead of left, right, top and bottom.

I don't believe this is quite right. The descent is measure.Bottom not measure.Y. I find that the following displays very close:

let x,y = 100.f,100.f
let bounds = SKRect(x+measure.Left,y+measure.Top,x+measure.Right,y+measure.Bottom)
let canvas = e.Surface.Canvas
mathpainter.Draw(canvas,x,y - float32 self.Height)
canvas.DrawRect(bounds, new SKPaint(Color = SKColor(0uy,0uy,0uy), Style = SKPaintStyle.Stroke))

image

The current behaviour here is good and what I would expect without documentation. Draw(canvas,x,y) draws into the rectangle measure translated by (x,y). (Aside from the y-coordinate bug above which requires subtracting float32 self.Height from the y coordinate of the draw method.)

@charlesroddie
Copy link
Collaborator

charlesroddie commented Jul 15, 2018

mathPainter.Draw seems to overwrite everything already on the canvas. E.g.

mathPainter.Draw(canvas, 0.f, 0.f)
e.Surface.Canvas.DrawCircle(0.f,0.f,100.f,new SKPaint(Color = SKColor(255uy,0uy,0uy)))

draws a red circle on a formula. But if the lines are swapped, the circle is not shown.

A SkiaMathPainter has a BackgroundColor property (which I think is not needed), but setting this to SKColor.Empty does not fix the problem of overwriting the canvas.

@charlesroddie
Copy link
Collaborator

charlesroddie commented Jul 15, 2018

@Happypig375 to summarize:

  • Draw overload taking an SKPoint (minor)
  • Draw y-coordinate incorrect offset
  • Draw clearing everything currently drawn on the canvas

Can you confirm these?

Also do you have an idea of what is causing .Net Native incompatibilities? I get various warnings and an error. Is this addressed in: dotnet/standard#295 ?

MCG : warning MCG0003: "COM and Windows Runtime interfaces require a GUID. Add System.Runtime.InteropServices.GuidAttribute(...) or Windows.Foundation.Metadata.GuidAttribute to interface...
error : MCG0024: MCG0024:UnresolvableTypeReference Unresolvable type reference 'System.Runtime.InteropServices.DefaultDllImportSearchPathsAttribute' in 'Assembly(Name=System.Runtime.InteropServices, Version=4.2.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a)' found. Please check the references in your build system. A reference is either missing or an assembly is missing an expected type.

@Happypig375
Copy link
Collaborator Author

Happypig375 commented Jul 16, 2018

@charlesroddie
Commit 949a2e4:

  • Draw(SKCanvas, SKPoint) overload added
  • Fixed y-coordinate to start top-left instead of bottom-left

However, I am not noticing any clears by SkiaMathPainter.

      e.Surface.Canvas.DrawCircle(0f, 0f, 100f, new global::SkiaSharp.SKPaint { Color = new global::SkiaSharp.SKColor(255, 0, 0) });
      painter.Draw(e.Surface.Canvas, (float)x, (float)y);
      e.Surface.Canvas.DrawCircle(300f, 0f, 100f, new global::SkiaSharp.SKPaint { Color = new global::SkiaSharp.SKColor(255, 255, 0) });

image
Both circles before and after the Draw operation are there, suggesting that Draw does not clear the canvas.

  • Fix the icon - it is currently bugged

@charlesroddie
Copy link
Collaborator

charlesroddie commented Jul 16, 2018

@Happypig375 What does this line do?

If that turns out to be the problem, can we have a pre3 please?

(Also onlooking through the file: I assume that CoordinatesFromBottomLeftInsteadOfTopLeft is marked for removal/cleanup later?)

@Happypig375
Copy link
Collaborator Author

Happypig375 commented Jul 17, 2018

@charlesroddie
That line blends the entire canvas with the background colour given. It has no effect when BackgroundColour is transparent, which is the default. Did you set it?

CoordinatesFromBottomLeftInsteadOfTopLeft is just experimental. Do you want to remove it?

@charlesroddie
Copy link
Collaborator

charlesroddie commented Jul 17, 2018

I am sure that line is causing the problem. canvas.DrawColor(SKColor.Empty,SKBlendMode.Overlay) turns the view gray which is exactly the problem I am getting with mathPainter.Draw.

Suggestion: Remove BackgroundColor, canvas.DrawColor, FillColor(). A MathPainter should not affect or interact with the canvas beyond the area into which it is drawing the LaTeX.

I think there is some cleanup to be done but we can do that after getting it working.

@charlesroddie
Copy link
Collaborator

@Happypig375 As soon as you can get pre3 nuget out without FillColor() we can release a beta incorporating CSharpMath. Thanks!

@Happypig375
Copy link
Collaborator Author

Now instead of having BackgroundColor, HighlightColor is now here.
It draws a filled rectangle behind the measured area to produce a "highlight effect" instead of using the weird method of FillColor.

Included in 0.1.0-pre3.

@Happypig375 Happypig375 changed the title CSharpMath 0.1.0-pre1 and 0.1.0-pre2 has been released! CSharpMath 0.1.0-pre1, 0.1.0-pre2 and 0.1.0-pre3 have been released! Jul 17, 2018
@charlesroddie
Copy link
Collaborator

charlesroddie commented Jul 17, 2018

Thanks a lot! Working well.

There is an issue with Measure now. measure.Bottom and measure.Top are switched and negated. So to get the true values you need let top, bottom = -measure.Bottom, -measure.Top. Height is OK.

@Happypig375
Copy link
Collaborator Author

Does Y have the same issue?

@charlesroddie
Copy link
Collaborator

I forgot the definition of Y. How does Y relate to Bottom and Top?

@Happypig375
Copy link
Collaborator Author

Fixed in latest commit!

@FoggyFinder
Copy link
Collaborator

@Happypig375 can you publish the new version, please?

@Happypig375
Copy link
Collaborator Author

0.1.0-pre4 released

@Happypig375 Happypig375 changed the title CSharpMath 0.1.0-pre1, 0.1.0-pre2 and 0.1.0-pre3 have been released! CSharpMath 0.1.0-pre1, 0.1.0-pre2, 0.1.0-pre3 and 0.1.0-pre4 have been released! Jul 21, 2018
@charlesroddie
Copy link
Collaborator

The .Net Native errors have changed to:

error : RHBIND : error RHB0011: Internal error: '!methodDesc->IsCanonicalMethod(CanonDefinitionTypeDiscoveryPolicy::AnyCanonLookup)' at 'f:\dd\ndp\rh\src\tools\rhbind\mdilmodule.cpp:12529'
error : ILT0005: 'C:\Users\Charles Roddie\.nuget\packages\runtime.win10-x64.microsoft.net.native.compiler\2.1.8\tools\x64\ilc\Tools\rhbind.exe @"C:\PROGRAMMING\GITHUB\CSharpMathMCVE\CsMathMcve\obj\x64\Release\ilc\intermediate\rhbindargs.CsMathMcve.rsp"' returned exit code 11

@Happypig375 do you think this is a Typography issue?

@Happypig375
Copy link
Collaborator Author

Happypig375 commented Jul 22, 2018

@charlesroddie
Nope, I pinned it down to the System.Numerics.Vectors package. That package caused this error:

Error: NUTC1056: Internal Compiler Error: 0x8000ffff. Error encountered while compiling method 'instance System.Boolean System.Numerics.Plane.Equals(System.Numerics.Plane)'.
Error: ILT0005: 'C:\Users\user\.nuget\packages\microsoft.net.native.compiler\1.7.3\tools\x86\ilc\Tools\nutc_driver.exe @"C:\Users\user\Source\Repos\NetNativeError\NetNativeError.UWP\obj\x86\Release\ilc\intermediate\MDIL\NetNativeError.UWP.rsp"' returned exit code 1

Now, I will try the .NET Core source and compiling it.

@Happypig375
Copy link
Collaborator Author

Welp, couldn't get the .NET Core source of System.Numerics.Vectors to compile either, even in Debug mode with the System.Numerics.Vectors.Performance.Tests project.

Error: Project 'C:\Users\user\Source\Repos\corefx\src\/System.Numerics.Vectors/ref/System.Numerics.Vectors.csproj' targets 'netcoreapp'. It cannot be referenced by a project that targets '_,Version=v0.0'.	System.Numerics.Vectors (src\System.Numerics.Vectors)	C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets	1656	

Guess an issue will be filed at corefx, then.

@charlesroddie
Copy link
Collaborator

CoreRT is probably a better place to post.

@Happypig375
Copy link
Collaborator Author

Opened as dotnet/corert#6131

@charlesroddie
Copy link
Collaborator

As far as I can tell from a quick search of the code, CSharpMath doesn't make significant use of System.Numerics.Vector, and Typography has it as an option SYSTEM_NUMERICS_VECTOR besides others.

@Happypig375
Copy link
Collaborator Author

I might just go with the Vector2 in Typography's source code. It is a polyfill for .NET Framework 2.0.

@Happypig375
Copy link
Collaborator Author

Merge into #15

@Happypig375 Happypig375 added the Resolution/Superceded The described announcement or pull request has been superceded. label Jul 27, 2018
@charlesroddie
Copy link
Collaborator

@Happypig375 By the way we released a beta using pre4 for iPad and Windows 10. Thanks for your work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution/Superceded The described announcement or pull request has been superceded. Type/Announcement
Projects
None yet
Development

No branches or pull requests

3 participants