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

About using native VirtualImageList #42

Closed carloBarazzetta closed 4 years ago

carloBarazzetta commented 4 years ago

Hi guys, in the last commit (August 18 - 22:44) I made some changes to TSVGIconImageCollection: now it inherits from TCustomImageCollection instead of TComponent and implements all required method (only from D10.3). With this change it is now possible to use a native VirtualImageList connected to the collection, without breaking the use with SVGIconVirtualImageList. To implement TSVGIconImageCollection.GetBitmap, I have moved bitmap creation from SVGIconImageListBase to SVGIconItems (maybe not the best place ...). I have some doubts if this modification can be useful, because the use of a native VirtualImageList does not bring particular advantages compared to the use of the SVGIconVirtualImageList (indeed it is not able to use Grayscale, Opacity, FixedColor ...). What do you think @pyscripter and @vincentparrett ? I also deleted SVGToIcon24 too ... do we need it?

vincentparrett commented 4 years ago

Do the component editors work correctly with this? That was the issue I saw when looking into this, as you pointed out, you have no control over the svg unique properties.

I will try to find some time today to have a play with this and see how it works.

pyscripter commented 4 years ago

This is BRILLIANT! MASTERSTROKE! It achieves many things:

Regarding FixedColor and GrayScale I can see two ways: a) add these properties to the SVGImageCollection or b) create a subclass of VirtualImageList with these properties. I think the first makes more sense.

There is a question whether SVGVirtualImageList as it stands is needed. It uses the TChangeScaleMessage to scale the bitmaps which I think is only available from Delphi 10.3 and VCL.VirtualImageList is available since 10.3. So the SVGIconImageList can be used with versions earlier than 10.3 and the SVGImageCollection/VirtualImageList combo with versions 10.3 or later.

carloBarazzetta commented 4 years ago

There is a question whether SVGVirtualImageList as it stands is needed. It uses the TChangeScaleMessage to scale the bitmaps which I think is only available from Delphi 10.3 and VCL.VirtualImageList is available since 10.3. So the SVGIconImageList can be used with versions earlier than 10.3 and the SVGImageCollection/VirtualImageList combo with versions 10.3 or later. SVGIconVirtualImageList uses the collection without re-defining the icons count and position to use. If you don't have Delphi 10.3 it's the only way to use a single collection on multiple form with different DPI so it make sense to maintain this component. The scaling can be easily make calling DPIChanged of the imagelist from OnAfterMonitorDPIChanged.

carloBarazzetta commented 4 years ago

Do the component editors work correctly with this? That was the issue I saw when looking into this, as you pointed out, you have no control over the svg unique properties.

image

I don't understand "have no control over the svg unique properties".

vincentparrett commented 4 years ago

I don't understand "have no control over the svg unique properties".

The Virtual ImageList Editor doesn't allow you to change FixedColor and GrayScale. I you move them to the Collection, that would then apply to all ImageList's hooked up to it. If you create a descendant TVirtualImageList to add those properties, they still won't appear on the component editor.

carloBarazzetta commented 4 years ago

The real problem is that TVirtualImageList.UpdateImageList is not "virtual", so I cannot change it to check if the collection was a TSVGIconImageCollection and then call a special TSVGIconImageCollection.GetBitmap with GrayScale and FixedColor params to obtain the correct image. :-( I'm searching a solution...

carloBarazzetta commented 4 years ago

But if we consider GrayScale and FixedColor additional attributes to the SVG icon (and in fact it is possible to define them in every single icon), we can decide that using the native VirtualImageList, it's not possible to change these attributes to all the icons of the imagelist, but he must do it for every single icon of the collection...

pyscripter commented 4 years ago

GrayScale and Fixed Color could be properties of item and SvgCollection to the same effect

pyscripter commented 4 years ago

Wasn't OnAfterMonitorDPIChanged introduced in 10.3.

carloBarazzetta commented 4 years ago

GrayScale and Fixed Color could be properties of item and SvgCollection to the same effect

Yes but don't resolve all the scenarios... Using a unique collection, for example with the "svg material design fonts collection" with Black color into SVG.... If you have a multiform, multimonitor application, using "per-style-control" (10.4) you can have this scenario:

carloBarazzetta commented 4 years ago

Wasn't OnAfterMonitorDPIChanged introduced in 10.3.

No, the event-handler it's available also in 10.1 and 10.2 but DPIChangedMessageHandler only from 10.3 ;-)

pyscripter commented 4 years ago

In this scenario the component made by @vincentparrett is more flexible vs native VirtualImageList because the FixedColor property is controlled on the imagelist.

I agree. Let's keep both. I don't see myself using two styles on the same app (defeats the purpose of styles), but others might. But in any case SvgCollection could get FixedColor and GrayScale properties.

vincentparrett commented 4 years ago

I agree. Let's keep both.

I Agree, both have their uses.

I don't see myself using two styles on the same app (defeats the purpose of styles), but others might.

I uses vcl styles but every day I wish I didn't. I can't imagine why anyone would want to use more than one. I think that feature was put in to support styles in the IDE form designer (not yet implemented). Sadly every time they mess with styles they make it worse!

But in any case SvgCollection could get FixedColor and GrayScale properties.

In that case, which one's would be used when rendering the bitmaps, the ones on the collection or those on the imagelist?

carloBarazzetta commented 4 years ago

I've committed now a new version with FixedColor and GrayScale properties available at collection level. The rule is:

pyscripter commented 4 years ago

Sounds good to me. I will check later tonight. If only you could add categories :)

I was planning to do some work on #13. Two SVG engines will be initially supported TSVG and Direct2X. The changes will be transparent, i.e. the components will work exactly as they do now.
@carloBarazzetta would you consider accepting such work?

carloBarazzetta commented 4 years ago

If only you could add categories :)

In the demo now I'm using a native VirtualImageList with some icons defined with "business" category... Do you want to extend this category at collection level?

carloBarazzetta commented 4 years ago

I was planning to do some work on #13. Two SVG engines will be initially supported TSVG and Direct2X. The changes will be transparent, i.e. the components will work exactly as they do now. @carloBarazzetta would you consider accepting such work?

Yes! I have not planned any works in the next days, now I am updating the "twin" project IconFontsImageList adding the same features of SVGImageList... If you think we have reached a good stability of those components we could release the official version 2.0. Then you can work on replacing the SVG engine (issue #13).

pyscripter commented 4 years ago

In the demo now I'm using a native VirtualImageList with some icons defined with "business" category... Do you want to extend this category at collection level?

Yes this does demonstrate that it is quite straight forward to implement categories. However what is needed is:

carloBarazzetta commented 4 years ago

I'm closing this issue: @vincentparrett did you also test with Delphi XE7? Everything should works...

vincentparrett commented 4 years ago

Sorry I haven't had chance yet, will try and look at it tomorrow.

carloBarazzetta commented 4 years ago

@vincentparrett: I'm ready to release official 2.0: can you give me a feed-back for backward compatibility with DXE7? Thanks.

vincentparrett commented 4 years ago

Sorry was just really busy.

So it doesn't compile in XE7, because AlphaColorToColor doesn't exist in System.UIConsts. I'm not sure when it was added?

vincentparrett commented 4 years ago
procedure CreateSVGColorList;
Var
  M: TMethod;
begin
  SVGColorList := TStringList.Create;
  SVGColorList.Options := SVGColorList.Options - [soUseLocale];

This fails in XE7 as Options doesn't exist on TStringList. No idea when that was added to TStringList?

vincentparrett commented 4 years ago
function ToGPRectF(R: TRect): TGPRectF;
begin
  with R do
    Result := WinApi.GDIPAPI.MakeRect(Single(Left), Top, Width, Height);
end;

Fails with an invalid typecast.

vincentparrett commented 4 years ago

There are lots of issues with XE7, I would imagine XE6 would have similar issues?

I also had to update the SVGIconPackage.dproj because the files were listed in the wrong folder (I guess you moved them?)

vincentparrett commented 4 years ago
  function InBounds(Item: TSVGObject): Boolean;
  var
    C: Integer;
    Bounds: TRectF;
  begin
    Result := True;
    if RectCount > 0 then
    begin
      for C := 0 to RectCount - 1 do
      begin
        Bounds := Item.ObjectBounds(True, True);
        if Bounds.IntersectsWith(Rects^[C]) then //<<<<
          Exit;
      end;
      Result := False;
    end;
  end;

Fails with [dcc32 Error] SVG.pas(2396): E2010 Incompatible types: 'TRectF' and 'TRect'

vincentparrett commented 4 years ago
procedure TSVG.ReadStyles(const Node: IXMLNode);
...
  Classes  := S.Split([SLineBreak]); //<<<

Fails with [dcc32 Error] SVG.pas(2470): E2250 There is no overloaded version of 'Split' that can be called with these arguments

carloBarazzetta commented 4 years ago
function ToGPRectF(R: TRect): TGPRectF;
begin
  with R do
    Result := WinApi.GDIPAPI.MakeRect(Single(Left), Top, Width, Height);
end;

Fails with an invalid typecast.

Retry now, because fails also in 10.1 and I've fixed.

carloBarazzetta commented 4 years ago

There are lots of issues with XE7, I would imagine XE6 would have similar issues?

I also had to update the SVGIconPackage.dproj because the files were listed in the wrong folder (I guess you moved them?)

Probably I've copied manually packages from DXE6 to DXE7 and change manually the differences: please fix the files for XE7.

I just changed PC and I haven't installed XE6 yet - I'll try later.

carloBarazzetta commented 4 years ago

I've made some minor code changes for compiling with XE8 and XE6. @pyscripter: please check that it doesn't break any functionality in the SVG library @vincentparrett: please try to compile with XE7, probably you must change only some $if compilerversion

vincentparrett commented 4 years ago

PR for XE7 pushed.. hope I haven't messed things up.. got a bit tangled with rebasing.

carloBarazzetta commented 4 years ago

PR for XE7 pushed.. hope I haven't messed things up.. got a bit tangled with rebasing.

Merged and fixed:

vincentparrett commented 4 years ago

I'm just changing the SVGHandler to SVGFactory - but I noticed the D2D SVG engine is now the default? Is that intended?

vincentparrett commented 4 years ago

@pyscripter I would suggest changing the define PreferNativeSvgSupport to UseD2DSVG or something a bit more explicit.

I would also suggest the built in SVG engine be the default, as not everyone is running windows 10 and this will catch out people.

Also, it would be nice to be able to change the engine at runtime based on the OS rather than using defines.

pyscripter commented 4 years ago

@carloBarazzetta Working late? 🥇

I noticed the D2D SVG engine is now the default? Is that intended? It is much faster in faster for complex SVGs such as the cowboy.svg and it might be more reliable , but it does not support the text element. It would take some benchmarking and testing to determine what should be the default on systems that have native svg support.

But currently the TSVG text handling is broken and I looking into it.

@carloBarazzetta Have you checked the FMX side?

vincentparrett commented 4 years ago

It would be nice to have a way to register a SVGFactory and then just specify which SVGFactory to use at runtime.

eg :

procedure RegisterSVGFactoryClass(const factoryClass : TSVGFactoryClass; const factoryID : string);

procedure SetSVGFactory(const factoryID : string) ;

This will make it possible to add new SVGFactory's in the future without modifying the core code, or messing with defines etc.

pyscripter commented 4 years ago

You can do that now. If you call SetGlobalSVGHandler at the initialization section of a unit that does the job. No need to recompile or change the include file.

For example to always use the PasSVG all you have to do is to call: SetGlobalSVGHandler(GetPasSVGHandler)

in the initialization of the Main form or whereever.

Similarly, if you create a new handler you do the same.

Why overengineer this?

pyscripter commented 4 years ago

I would also suggest the built in SVG engine be the default, as not everyone is running windows 10 and this will catch out people.

Have a look at the function GlobalSVGHandler. It only uses the native Windows SVG support, if it is available and as I mentioned above it can be easily overwritten without changing conditional defines with just one line in the initialization section of the Main Form. In any case feel free to change the default.

vincentparrett commented 4 years ago

Ah, I didn't notice that before.

Still, the default behavior is now use D@D on win 10, otherwise use pas version.. I think the svg engine should be an explicit choice by the developer.

pyscripter commented 4 years ago

Default means what happens if the user provides no choice. :) The choice cannot be a component property, since you cannot change the Factory during or after the component is loaded or at design time. It will be documented that you could use a specific a factory by calling SetGlobalSVGFactory at the initialization section of any unit. Or by changing a conditional define.

carloBarazzetta commented 4 years ago

Discussion continues on issue #66