Design options for background-bitmap cache invalidation vs. public Background.OnChange

[zencode1]

This is a discussion for PR #1402 on how to resolve concerns about utilizing Background.OnChange to determine when the cached image should be reprepared.

Problem

The recent background-bitmap caching work (FBackgroundPrepared) invalidates its cached bitmap by hooking Background.OnChange. Since Background is a public property and OnChange is a single-slot TNotifyEvent, any consumer that does Tree.Background.OnChange := MyHandler after construction silently replaces the tree's internal hook. Subsequent background changes then leave the cached bitmap stale, and the tree keeps painting the old image.

Options

Posted below as separate comments so each can be discussed/reacted-to independently:

1. Multicast wrapper - subclass TVTBackground to support multiple independent change listeners, leaving OnChange entirely free for consumers.
2. Public InvalidateBackground method - consumers call it manually after modifying Background.
3. New OnBackgroundChange event - internal hook stays as-is; document Background.OnChange as reserved.
4. Same as 3, plus runtime detection/chaining if Background.OnChange gets overwritten.
5. Same as 4, but silent - no new public event; just detect-and-chain.

Feedback on which direction to take (or a different one entirely) is welcome before I put together an implementation.

[zencode1]

Option 1: Alter TVTBackground to support multiple change listeners, leaving OnChange free for consumers

Pros: Fixes the root cause rather than working around it. Existing code that assigns Background.OnChange keeps working unchanged, so nothing breaks for current consumers. Matches the pattern already used elsewhere e.g. TCustomImageList.RegisterChanges/UnregisterChanges.

Cons: Turns TVTBackground from a plain type alias (TPicture/TBitmap) into a real subclass, so the change has to be implemented once for the VCL branch (TPicture) and once for the FMX branch (TBitmap).

type
  { TVTBackground }
  TVTBackground = class({$IFDEF VT_FMX}TBitmap{$ELSE}TPicture{$ENDIF})
  private
    FChangeHandlers: TList<TNotifyEvent>;
    procedure DispatchChanges(Sender: TObject);
  protected
    {$IFDEF VT_FMX}
    procedure BitmapChanged; override;
    {$ELSE}
    procedure Changed(Sender: TObject); override;
    {$ENDIF}
  public
    constructor Create;
    destructor Destroy; override;
    procedure RegisterChanges(NotifyEvent: TNotifyEvent);
    procedure UnregisterChanges(NotifyEvent: TNotifyEvent);
  end;

{ TVTBackground }

constructor TVTBackground.Create;
begin
  inherited Create;
  FChangeHandlers := TList<TNotifyEvent>.Create;
end;

destructor TVTBackground.Destroy;
begin
  FChangeHandlers.Free;
  inherited Destroy;
end;

{$IFDEF VT_FMX}
procedure TVTBackground.BitmapChanged;
begin
  inherited BitmapChanged; // fires OnChange for compatibility
  DispatchChanges(Self);
end;
{$ELSE}
procedure TVTBackground.Changed(Sender: TObject);
begin
  inherited Changed(Sender); // fires OnChange for compatibility
  DispatchChanges(Sender);
end;
{$ENDIF}

procedure TVTBackground.DispatchChanges(Sender: TObject);
var
  Handler: TNotifyEvent;
begin
  for Handler in FChangeHandlers do
    if Assigned(Handler) then
      Handler(Sender);
end;

procedure TVTBackground.RegisterChanges(NotifyEvent: TNotifyEvent);
begin
  if FChangeHandlers.IndexOf(NotifyEvent) < 0 then
    FChangeHandlers.Add(NotifyEvent);
end;

procedure TVTBackground.UnregisterChanges(NotifyEvent: TNotifyEvent);
begin
  FChangeHandlers.Remove(NotifyEvent);
end;

constructor TBaseVirtualTree.Create;
begin
  //...
  FBackground := TVTBackground.Create;
  FBackground.OnChange := BackgroundPictureChanged;
  //...
end;

destructor TBaseVirtualTree.Destroy;
begin
  //...
  FBackground.UnregisterChanges(BackgroundPictureChanged);
  FBackground.Free;
  //...
end;

[zencode1]

Option 2: Publicly expose a method for consumers to call as a workaround (BackgroundChanged, InvalidateBackground, etc.)

Pros: Simple solution and easily resolves the issue. Also has the benefit of resolving the comment in BackgroundPictureChanged by allowing the consumer to notify the tree of any modifications (including direct pixel edits that bypass Changed/OnChange entirely).

Cons: Consumers that hook into Background.OnChange must be modified to also call this. If the consumer doesn't call the new method, the background bitmap cache stays stale - leaving us in the same position we'd be in without any fix at all.

public
  procedure InvalidateBackground;

procedure TBaseVirtualTree.InvalidateBackground;
begin
  FreeAndNil(FBackgroundPrepared.Bitmap);
  Invalidate;
end;

[zencode1]

Option 3: Keep the internal hook as-is. Add a new event handler for consumers to use (OnBackgroundChange), and document that direct assignment to Background.OnChange is not allowed

Pros: Simple solution and easily resolves the issue for anyone who switches to the new event.

Cons: Doesn't actually fix anything for existing programs that already hook Background.OnChange directly - nothing breaks (they behave exactly as before this PR), but they silently keep the original stale-cache bug, since we have no way to detect or warn them that they're on the unsupported path. Relies entirely on documentation/changelog to get consumers to migrate.

private
  FOnBackgroundChange: TNotifyEvent;

published
  property OnBackgroundChange: TNotifyEvent read FOnBackgroundChange write FOnBackgroundChange;

procedure TBaseVirtualTree.BackgroundPictureChanged(...);
begin
  //...
  if Assigned(FOnBackgroundChange) then
    FOnBackgroundChange(Sender);
end;

[zencode1]

Option 4: Same as 3, but enforce this requirement when painting by detecting if Background.OnChange has been overwritten. If so, reassign internally and chain the handlers.

Pros: Easily resolves the issue with no disruption to existing consumers.

Cons: Tiny added overhead to TBaseVirtualTree.GetBackgroundBitmap to check and correct the event handler

private
  FOnBackgroundChange: TNotifyEvent;

published
  property OnBackgroundChange: TNotifyEvent read FOnBackgroundChange write FOnBackgroundChange;

procedure TBaseVirtualTree.GetBackgroundBitmap(...);
begin
  if Background.OnChange <> BackgroundPictureChanged then
  begin
    // Background.OnChange was hijacked, reassign and chain the handler
    FOnBackgroundChange := Background.OnChange;
    Background.OnChange := BackgroundPictureChanged;
  end;
  //...
end;

procedure TBaseVirtualTree.BackgroundPictureChanged(...);
begin
  //...
  if Assigned(FOnBackgroundChange) then
    FOnBackgroundChange(Sender);
end;

[zencode1]

Option 5: Same as 4 except don't expose the new handler. Detect if Background.OnChange has been overwritten. Silently chain the handlers

Pros: Easily resolves the issue with no disruption to existing consumers.

Cons: Tiny added overhead to TBaseVirtualTree.GetBackgroundBitmap to check and correct the event handler.

private
  FCustomBackgroundChange: TNotifyEvent;

procedure TBaseVirtualTree.GetBackgroundBitmap(...);
begin
  if Background.OnChange <> BackgroundPictureChanged then
  begin
    // Background.OnChange was hijacked, reassign and chain the handler
    FCustomBackgroundChange := Background.OnChange;
    Background.OnChange := BackgroundPictureChanged;
  end;
  //...
end;

procedure TBaseVirtualTree.BackgroundPictureChanged(...);
begin
  //...
  if Assigned(FCustomBackgroundChange) then
    FCustomBackgroundChange(Sender);
end;