EtheaDev / SVGIconImageList

Three engines to render SVG (Delphi Image32, Skia4Delphi, Direct2D wrapper) and four components to simplify use of SVG images (resize, fixedcolor, grayscale...)
Apache License 2.0
321 stars 93 forks source link

Skia4Delphi support #201

Closed carloBarazzetta closed 2 years ago

carloBarazzetta commented 2 years ago

I've added a beta version of this new engine: https://github.com/viniciusfbb/skia4delphi Some svg files are not correctly rendered (for example cowboy and Butterfly...) FixedColor don't work, GrayScale is not supported and the rendering area is not "proportional"... I've added the new factory to the Benchmark and using a complex svg like lion.svg the engine is very fast... Feedbacks are welcome...

pyscripter commented 2 years ago

Sounds good. A drawback is that skia svg does not support the style and class attribute https://bugs.chromium.org/p/skia/issues/detail?id=12251&q=style&can=2. It does not look like such support is coming soon (low priority).

pyscripter commented 2 years ago

There are appear to be a number of serious issues in skia svg support (see https://bugs.chromium.org/p/skia/issues/list?q=svg&can=2&colspec=ID%20Type%20Status%20Priority%20M%20Area%20Owner%20Summary%20Opened) for details:

Also, although the ski svg includes accessing the svg elements via DOM. the Delphi wrapper does not include the relevant parts.

My suggestion is to leave out of the master for now (create a branch) and get back to it when it is more mature. Other parts of skia are great but not the svg support.

carloBarazzetta commented 2 years ago

Yes, I agree with you, I will revert to previous version of the repo and make a branch... probably at the end of the week... Now I'm very busy...

viniciusfbb commented 2 years ago

Hello guys! First, I want to congratulate you on the beautiful work! It's great to have an svg library that implements and standardizes the use of different renderers. It's cool even for performance comparison purposes.

Regarding DOM access to SVG elements (which appears in the skia documentation), it was only introduced in the m93 version, so you won't find it in our wrapper (which uses the m88 version). The initial purpose was not to give access to the elements, as you can already change size, change colors through filters, etc. The documentation on the google website is always from the latest released version.

About the limitation, it's true that skia's SVG still doesn't fully support CSS (consequently style element and style property), I described this limitation earlier https://github.com/viniciusfbb/skia4delphi/blob/main/Documents/SVG.md#limitations However this does not affect the use of the vast majority of icons set. Just to show the almost irrelevance of this limitation, in flutter itself skia is also used to render svg, and they even describe this limitation: https://github.com/dnfield/flutter_svg#recommended-adobe-illustrator-svg-configuration

I have to disagree about the bugs. The skia SVG was in experimental phase for years and the official release was released less than 1 year ago, in the m88 version, (the version that Skia4Delphi uses). There are very few reports after the release of the official release.

Anyway, I also recommend not implementing it yet, because the current version (components and samples) of Skia4Delphi, renders with CPU raster, the performance would be worse than current implementations. However, in the next version we will already make available the rendering via GPU, which is much faster than the current one.

Yours sincerely,

pyscripter commented 2 years ago

@viniciusfbb Thanks for your input. Skia4Delphi is great work by the way. Would it be too much to ask you to add the SVG DOM stuff at some future version of your library? It does give a lot more control (e.g. FixedColor, ApplyFixedColorToRootOnly properties of this library) and opens other possibilities.

viniciusfbb commented 2 years ago

Not within a month. Current priority is to upgrade to m89 and implement hardware accelerator of render. In future version, yes.

viniciusfbb commented 2 years ago

Just an update. We have already decided to upgrade to m93 and we will probably already expose the svg elements, along with the implementation using the GPU as early as the next release (within 1 month). I believe that in this next version the performance will be much better than the other renders as neither Cairos nor Image32 use hardware accelerator.

pyscripter commented 2 years ago

Great!
But regarding GPU acceleration don't hold your breath. For painting simple svg files for example it does not make much difference. D2D offers GPU acceleration but it was much slower than without. In SVGIconImageList it is disabled by default. GPU acceleration makes a huge difference in complex animations such as the ones you see in computer games. But I may be wrong. We' ll see.

viniciusfbb commented 2 years ago

@pyscripter In the current D2D implementation that's in the repository, you're painting on the GPU (using D2D) and bringing the image from GPU memory to the CPU memory by transforming into vcl's TBitmap, this switch from GPU to CPU is extremely costly. Our idea would be to render via the GPU directly on the screen, it will probably be twice as fast.

carloBarazzetta commented 2 years ago

Thank you @viniciusfbb for your contribution and great work on Skia4Delphi.. I have been very busy these days and I have not been able to contribute to the discussion. I decided to keep the current implementation on the trunk of the project, even if the library has some limitations it is already usable, I just have to correct an aspect-ratio problem on the rendering of non-square surfaces. In terms of performance, it already seems good to me!

carloBarazzetta commented 2 years ago

Today I've committed a new version compatible with SKIA4Delphi 3.0 but many SVG files are not correctly rendered if includes "DOCTYPE" section, like explained here

carloBarazzetta commented 2 years ago

Problems for other set of icons, here

carloBarazzetta commented 2 years ago

Updated support to SKIA4Delphi 3.0.3 and fixed some rendering problems.

birbilis commented 2 years ago

Does Skia4Delphi cause any deployment dependencies (for Windows and other platforms)?

carloBarazzetta commented 2 years ago

Actually SKIA4Delphi support is available only for VCL version of SVGIconImageList. You must deploy the proper .dll ok SKIA with your project, as explained in SKIA project.

birbilis commented 2 years ago

thanks, was hoping they could statically link somehow

paulocesarbot commented 2 years ago

Hi! As it is a C++ library, static linking on Windows is a little more complex, because it involves extracting STL object files, as well as mangling names. But we'll work on that later, even to make it easier to use and compress better the output, although it's not an urgent need. Thank you for ask! 😁

birbilis commented 2 years ago

As in .NET one uses Microsoft C++/CLI for such cases, I'd expect to be able to use C++Builder to make some shim for Delphi to talk to (to handle the name manging etc. internally automatically)

viniciusfbb commented 2 years ago

@carloBarazzetta Improved Ethea compatibility by adding GrayScale property to TSkSvgBrush: v3.1.0

carloBarazzetta commented 2 years ago

Thank you @viniciusfbb! We need only a final implementation, ApplyFixedColorToRootOnly, very useful as explained here: https://github.com/EtheaDev/SVGIconImageList/issues/161 In TSkSvgBrush could be ApplyOverrideColorToRootOnly...

viniciusfbb commented 2 years ago

Thakyou @viniciusfbb! We need only a final implementation, ApplyFixedColorToRootOnly, very useful as explained here: #161 In TSkSvgBrush could be ApplyOverrideColorToRootOnly...

Right! In the next release we will add this.

carloBarazzetta commented 2 years ago

I have some problem with SKIA4Delphi in FMX: I have implemented the TFmxImageSkiaSVG class in the FMX.ImageSkiaSVG unit to render an SVG icon in FMX using SKIA, using the Bitmap.SkiaDraw helper... I don't know why the bitmaps are never visible using my SVGIconImageList component (SVGIconImageListDemoFMX.dpr demo) and is only visible when clicking on the next icon using my TSVGIconImage component (SVGIconImageDemoFMX demo). If someone can help me, remember to activate {$ DEFINE Skia_SVGEngine} in SVGIconImageList.inc instead of Image32_SVGEngine... to activate SKIA support. I'm not an FMX platform expert and I'm probably missing out something... Thank's

viniciusfbb commented 2 years ago

@carloBarazzetta I looked at the code and I suggest some changes:

1.

Change the line: https://github.com/EtheaDev/SVGIconImageList/blob/e451a1ded23d48ba0bd3d80335cd36d886f1af08/Source/FMX.ImageSkiaSVG.pas#L109 to:

  FSvg.Source := Source;

2.

To improve performance, remove the line: https://github.com/EtheaDev/SVGIconImageList/blob/e451a1ded23d48ba0bd3d80335cd36d886f1af08/Source/FMX.ImageSkiaSVG.pas#L152 And remove the , False from the line: https://github.com/EtheaDev/SVGIconImageList/blob/e451a1ded23d48ba0bd3d80335cd36d886f1af08/Source/FMX.ImageSkiaSVG.pas#L155

3.

At line: https://github.com/EtheaDev/SVGIconImageList/blob/e451a1ded23d48ba0bd3d80335cd36d886f1af08/Source/FMX.SVGIconImageList.pas#L259-L265 I suggest to change the block (BeginScene, Clear and EndScene) to each implementation of "PaintToBitmap" because the "PaintToBitmap" method of Skia4Delphi svg engine, can't use it.

4.

At line: https://github.com/EtheaDev/SVGIconImageList/blob/e451a1ded23d48ba0bd3d80335cd36d886f1af08/Source/FMX.SVGIconImageList.pas#L293-L294 To improve performance, I suggest change it to:

  LBitmap.SetSize(LBitmapWidth, LBitmapHeight);
carloBarazzetta commented 2 years ago

Thank you very much Vinicius, now works perfectly!

viniciusfbb commented 2 years ago

@carloBarazzetta, we decided not to directly add ApplyFixedColorToRootOnly to our main code, as our idea is to keep the components short as much as possible. However in our new version released today, we improved the TSkSvgBrush code to allow for more customizations.

I made an example code of how to create a descendant of TSkSvgBrush with this new property. The code works for both, FMX and Vcl:

unit Unit1;

interface

uses
  System.UITypes, Skia, Skia.FMX; // Or Skia.Vcl in case of Vcl

type
  { TSkSvgBrushEx }

  TSkSvgBrushEx = class(TSkSvgBrush)
  strict private
    FOverrideRootColor: TAlphaColor;
    procedure SetOverrideRootColor(const AValue: TAlphaColor);
  strict protected
    procedure DoAssign(ASource: TSkSvgBrush); override;
    function MakeDOM: ISkSVGDOM; override;
  public
    function Equals(AObject: TObject): Boolean; override;
  published
    property OverrideRootColor: TAlphaColor read FOverrideRootColor write SetOverrideRootColor default TAlphaColors.Null;
  end;

implementation

uses
  System.SysUtils;

{$IF CompilerVersion < 29}
type
  { TSkFormatSettingsHelper }

  TSkFormatSettingsHelper = record helper for TFormatSettings
  strict private
    class var
      FInvariant: TFormatSettings;
    class constructor Create;
  public
    class property Invariant: TFormatSettings read FInvariant;
  end;

class constructor TSkFormatSettingsHelper.Create;
begin
  FInvariant := TFormatSettings.Create('en-US');
end;
{$ENDIF}

{ TSkSvgBrushEx }

procedure TSkSvgBrushEx.DoAssign(ASource: TSkSvgBrush);
begin
  if ASource is TSkSvgBrushEx then
    FOverrideRootColor := TSkSvgBrushEx(ASource).FOverrideRootColor
  else
    FOverrideRootColor := TAlphaColors.Null;
  inherited;
end;

function TSkSvgBrushEx.Equals(AObject: TObject): Boolean;
begin
  Result := (AObject is TSkSvgBrushEx) and (FOverrideRootColor = TSkSvgBrushEx(AObject).FOverrideRootColor) and inherited;
end;

function TSkSvgBrushEx.MakeDOM: ISkSVGDOM;
var
  LAlphaColorRec: TAlphaColorRec;
  LNewColor: string;
begin
  Result := inherited;
  if FOverrideRootColor <> TAlphaColors.Null then
  begin
    LAlphaColorRec := TAlphaColorRec(FOverrideRootColor);
    LNewColor := Format('rgb(%d,%d,%d)', [LAlphaColorRec.R, LAlphaColorRec.G, LAlphaColorRec.B]);
    if Result.Root.TrySetAttribute('fill', LNewColor) and Result.Root.TrySetAttribute('color', LNewColor) and (LAlphaColorRec.A <> High(LAlphaColorRec.A)) then
      Result.Root.TrySetAttribute('opacity', FloatToStr(LAlphaColorRec.A / High(LAlphaColorRec.A), TFormatSettings.Invariant));
  end;
end;

procedure TSkSvgBrushEx.SetOverrideRootColor(const AValue: TAlphaColor);
begin
  if FOverrideRootColor <> AValue then
  begin
    FOverrideRootColor := AValue;
    RecreateDOM;
    if HasContent then
      DoChanged;
  end;
end;

end.
carloBarazzetta commented 2 years ago

Thank's to @viniciusfbb I've completed the support for SKIA4Delphi, adding ApplyFixedColorToRootOnly.