-
-
Notifications
You must be signed in to change notification settings - Fork 147
CreateAsInterface proposal #408
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
base: main
Are you sure you want to change the base?
Conversation
# Skia4Delphi `AsInterface` Proposal & Demo Most of the Skia API is exposed as interfaces. This takes advantage of reference counting, which is nice to have. Usually, the class has limited public methods and most functionality is exposed through the interface. **Example:** ```Delphi type { ISkPaint } ISkPaint = interface(ISkObject) ['{C95825F8-0D51-4BCE-8945-84AFE6264213}'] function GetAlpha: Byte; function GetAlphaF: Single; { ... 50 other PUBLIC members ...} property StrokeWidth: Single read GetStrokeWidth write SetStrokeWidth; property Style: TSkPaintStyle read GetStyle write SetStyle; end; { TSkPaint } TSkPaint = class(TSkObject, ISkPaint) strict private function GetAlpha: Byte; function GetAlphaF: Single; { ... 34 other STRICT PRIVATE memebers ... } procedure SetStrokeWidth(const AValue: Single); procedure SetStyle(const AValue: TSkPaintStyle); public constructor Create; overload; constructor Create(const APaint: ISkPaint); overload; constructor Create(const AStyle: TSkPaintStyle); overload; end; ``` This is fine for *traditional* variable declarations: ```Delphi var paint: ISkPaint; begin paint := TSkPaint.Create; paint.Color := TAlphaColors.Darkorange; end; ``` Unfortunately, it is *incompatible* with **type inferance**: ```Delphi begin var inferred := TSkPaint.Create; inferred.Color := TAlphaColors.Darkorange; // E2003 Undeclared identifier: 'Color' end; ``` Forces the more verbose form of inline variable declaration: ```Delphi begin var explicit: ISkPaint := TSkPaint.Create(); explicit.Color := TAlphaColors.Darkorange; end; ``` The proposed `CreateAsInterace` methods fix this: ```Delphi begin var better := TSkPaint.Create(); better.Color := TAlphaColors.Darkorange; end; ``` Simply duplicate the existing constructors: ```Delphi public constructor Create; overload; constructor Create(const APaint: ISkPaint); overload; constructor Create(const AStyle: TSkPaintStyle); overload; end; ``` Making them a `class function` named `CreateAsInterface` with the same arguements, and returning the `interface`: ```Delphi public class function CreateAsInterface: ISkPaint; overload; class function CreateAsInterface(const APaint: ISkPaint): ISkPaint; overload; class function CreateAsInterface(const AStyle: TSkPaintStyle): ISkPaint; overload; end; ``` Any name could be used (`Init`, `AsInterface`, `George`, etc.), but this one makes them very visible in class completion to aid in discovery. The implementation just calls the existing constructors, but since the Result is explicitly the Interface, this works as expected: ```Delphi implementation { TSkPaint } class function TSkPaint.CreateAsInterface: ISkPaint; begin Result := TSkPaint.Create(); end; class function TSkPaint.CreateAsInterface(const APaint: ISkPaint): ISkPaint; begin Result := TSkPaint.Create(APaint); end; class function TSkPaint.CreateAsInterface(const AStyle: TSkPaintStyle): ISkPaint; begin Result := TSkPaint.Create(AStyle); end; ``` Type inferance is as preferable. Beyond less typing it makes code more maintainable (type only needs to be changed in one place) and reduces errors resulting from mismatched types and constructors. The easier it is to take advantage of this feature in Delphi the better.
C:\Users\jim\Documents\GitHub\skia4delphi\Samples\CreateAsInterface
Excellent suggestion and it makes a lot sense too. It gets my vote. Nice work @jimmckeeth |
Hi @jimmckeeth, Thank you very much for all your contributions — the project likely wouldn’t be nearly as relevant without you. I removed some units from the PR.I’ve excluded the Regarding the changes, I agree that the var LPaint := TSkPaint.Create;
Foo(LPaint); // Foo(APaint: ISkPaint)
LPaint.Reset; // Access violation The compiler implicitly casts In Google Skia, Since the current implementation leads to incorrect behavior, we’re left with a few options:
The only option that preserves backward compatibility is the one currently proposed, so LGTM. I just have two recommendations:
For example: public
constructor Create; overload; deprecated 'Use TSkPaint.Make instead.';
constructor Create(const APaint: ISkPaint); overload; deprecated 'Use TSkPaint.MakeCopy instead.';
constructor Create(const AStyle: TSkPaintStyle); overload; deprecated 'Use TSkPaint.Make instead.';
class function Make: ISkPaint; overload; static;
class function Make(const AStyle: TSkPaintStyle): ISkPaint; overload; static;
class function MakeCopy(const APaint: ISkPaint): ISkPaint; static; @paulocesarbot What do you think? |
LGTM. Best regards, |
and implemented on other classes * TSkColorFilter - For color transformations * TSkFont - Font rendering and text metrics handling * TSkPaint - Graphics styling configuration including colors, strokes, and effects * TSkParagraphBuilder - A builder class for creating formatted text paragraphs with advanced text layout features like styles, fonts, and text decorations * TSkParagraphStyle - Styling configuration for paragraph layouts * TSkPath - Vector path definition and manipulation * TSkPathBuilder - Used for building paths incrementally * TSkPathMeasure - For measuring and extracting information about paths * TSkPictureRecorder - Used for recording drawing commands * TSkPixmap - For pixel buffer access * TSkRegion - Represents a 2D set of pixels for clipping or hit-testing * TSkRoundRect - Used for rounded rectangles * TSkRuntimeBlenderBuilder - A builder class for creating custom blend modes at runtime, enabling custom pixel blending operations * TSkRuntimeShaderBuilder - A builder class for creating custom shaders at runtime, allowing dynamic generation of graphical effects and patterns * TSkShaper - Text shaping engine for complex script rendering * TSkString - String handling utilities for Skia * TSkStrutStyle - Text layout configuration for line spacing and alignment * TSkTextStyle - Text styling configuration for paragraphs * TSkUnicode - Unicode text processing and manipulation utilities for international text handling
Renaming them as .make works. Sorry about including the other files. I thought I excluded them. Thanks for catching that. I've added the changes you suggested and to the other classes, and disabled the warning where it occured. Modified classes
|
Skia4Delphi
CreateAsInterface
Proposal & DemoMost of the Skia API is exposed as interfaces. This takes advantage of reference counting, which is nice to have. Usually, the class has limited public methods and most functionality is exposed through the interface. Here is the declaration for
TSkPaint
andISkPaint
for example.This is fine for traditional variable declarations:
Unfortunately, it is incompatible with type inferance:
Forces the more verbose form of inline variable declaration:
The proposed
CreateAsInterace
methods fix this:Simply duplicate the existing constructors:
Making them a
class function
namedCreateAsInterface
with the same arguments, and returning theinterface
:Any name could be used (
Init
,AsInterface
,George
, etc.), but this one makes them very visible in class completion to aid in discovery.The implementation just calls the existing constructors, but since the Result is explicitly the Interface, this works as expected:
Type inference is as preferable. Beyond less typing it makes code more maintainable (type only needs to be changed in one place) and reduces errors resulting from mismatched types and constructors. The easier it is to take advantage of this feature in Delphi the better.