Bitmap zurück geben und freigeben

[Desmulator]

Also es geht um folgenden Code:

Delphi-Quellcode:

   Image := TBitmap.Create;    



   ...

    //Zurückgeben
    Result := Image;

  finally
    //Image frei geben
    Image.FreeImage;
    Image.Free;
  end;

Das Problem ist , dass ich als rückgabe eine leere Bitmap erhalte, und wenn ich Image im finally block nicht frei gebe klappt alles, bis auf das ich nach ner zeit keinen speicher mehr habe...

Was soll/muss ich anders machne?

MfG

[DeddyH]

Übergib das Image doch als Parameter.

Delphi-Quellcode:

procedure TuWas(const Image: TBitmap);
begin
  //hier Dein Code
end;

procedure TFromBla.Blubb;
var Image: TBitmap;
begin
  Image := TBitmap.Create;
  try 
    Image.Width := 100;
    Image.Height := 100;
    TuWas(Image);
  finally
    Image.Free;
  end;
end;

[Desmulator]

ich würde es aber gerne so nutzen

Delphi-Quellcode:

GameScreen.Canvas.Draw(StaticObject.PosX,StaticObject.PosY,StaticObject.Appear());

TStaticObject.Appear() gibt eine Bitmap zurück die ich auf GameScreen zeichen will...

ich müsste es nun alles umständlich mit einer weiteren tempbmp lösen

Delphi-Quellcode:

TStaticObject.Appear(TempBmp);
GameScreen.Canvas.Draw(StaticObject.PosX,StaticObject.PosY,TempBmp); //Das finde ich is doof

[DeddyH]

Lies Dir mal diesen Thread durch.

[Muetze1]

Zitat:

Zitat von Desmulator

ich würde es aber gerne so nutzen

Delphi-Quellcode:

GameScreen.Canvas.Draw(StaticObject.PosX,StaticObject.PosY,StaticObject.Appear());

TStaticObject.Appear() gibt eine Bitmap zurück die ich auf GameScreen zeichen will...

Dann musst du mit dem Speicherleck leben. Grundsätzlich gibt man Objekte auf der Ebene frei, wo sie angelegt wurden. Da du das Objekt aber zurück geben willst, kannst du das nicht machen. Also entweder zusätzliche Bitmap oder Speicherleck.

Anderer Vorschlag: Warum nicht so?

Delphi-Quellcode:

  StaticObject.DrawAppear(GameScreen.Canvas);

Schliesslich hat StaticObject PosX, PosY und das Bitmap...

Nachtrag zu deiner Lösung: (freigeben fehlte, try/finally nur Sicherheit)

Delphi-Quellcode:

TStaticObject.Appear(TempBmp);
try
  GameScreen.Canvas.Draw(StaticObject.PosX,StaticObject.PosY,TempBmp); //Das finde ich is doof
finally
  TempBmp.Free;
end;

[Desmulator]

Zitat:

Zitat von Muetze1

Delphi-Quellcode:

  StaticObject.DrawAppear(GameScreen.Canvas);

Schliesslich hat StaticObject PosX, PosY und das Bitmap...

Naja ich möchte die Objecte auf den GameScreen zeichnen und nicht das sich die Objecte auf den GameScreen Zeichnen^^

also ich mach es dann gezwungener masen mit einem tmp bmp...

thx

[Desmulator]

Delphi-Quellcode:

  //Wir zeichenen das statische Onject auf seine Position
  TempBmp := TBitmap.Create;
  StaticObject.Appear(TempBmp);
  try
    GameScreen.Canvas.Draw(StaticObject.PosX,StaticObject.PosY,TempBmp);
  finally
    TempBmp.Free;
  end;

Ich erhalte eine zugriffsverletzung und zwar beim zeichen auf den GameScreen ( Paintbox )

wieso das jetzz?

[DeddyH]

Hmm... kann sein, dass ich jetzt Stuss schreibe, aber könnte es so gehen?

Delphi-Quellcode:

  TempBmp := StaticObject.Appear;
  try
    GameScreen.Canvas.Draw(StaticObject.PosX,StaticObject.PosY,TempBmp);
  finally
    TempBmp.Free;
  end;

[Muetze1]

Zitat:

Zitat von Desmulator

Delphi-Quellcode:

  //Wir zeichenen das statische Onject auf seine Position
  TempBmp := TBitmap.Create;
  StaticObject.Appear(TempBmp);
  try
    GameScreen.Canvas.Draw(StaticObject.PosX,StaticObject.PosY,TempBmp);
  finally
    TempBmp.Free;
  end;

Ich erhalte eine zugriffsverletzung und zwar beim zeichen auf den GameScreen ( Paintbox )

wieso das jetzz?

Was machst du denn nun in der Appear() Methode? Und warum steht der Aufruf der selben ausserhalb des try/finally Abschnittes?

[Desmulator]

Code von Appear

Delphi-Quellcode:

function TStaticObject.Appear( var Dest: TBitmap ) : TBitMap;
var
  Image : TBitmap;
begin
  try
    //Image erstellen
    Image := TBitmap.Create;

    { ... unwichtig ... }

    //"Zurück geben"
    Dest := Image;

  finally
    //Image frei geben
    Image.FreeImage;
    Image.Free;
  end;
end;

Ich glaube der Fehle liegt bei Dest := Image; ka obs stimmt

[Muetze1]

Junge, watt machst du denn da...

Du legst ein Bitmap an, gibst es an die Funktion Appear(), wo du erneut ein Bitmap anlegst. Dann trittst du das übergebene Bitmap mit den Füßen und weist das lokale zu.

Also: Wir hatten doch extra vorgeschlagen das Bitmap zu übergeben, damit du in der Funktion nichts mehr anlegen oder freigeben musst - weil halt genau dieses alles nach außen gelegt wurde...

Delphi-Quellcode:

function TStaticObject.Appear( const Dest: TBitmap ) : TBitMap;
begin
    // nicht unwichtig, sondern alles auf Dest machen!
    
end;

[Desmulator]

jawohl ich habe verstanden

Thx

Edit: Es geht aber trozdem erhalte ich die meldung das mit die ressoucen ausgehen

[Muetze1]

Also, wenn ich davon ausgehe, dass die bisher besprochene Funktion nun so wie geraten ist, dann solltest du ein anderes Leck an einer anderen Stelle haben...

[DeddyH]

Zitat:

Zitat von Muetze1

Delphi-Quellcode:

function TStaticObject.Appear( const Dest: TBitmap ) : TBitMap;

Ich denke, eher so:

Delphi-Quellcode:

procedure TStaticObject.Appear( const Dest: TBitmap );

Der Rückgabewert hat sich somit erledigt, oder mache ich jetzt einen Denkfehler?

[Lossy eX]

Mal ganz blöd gefragt. Warum musst du eigentlich jedes Mal ein Bild anlegen, wenn du das Objekt zeichnen willst? Wäre es nicht besser, wenn das StaticObjekt sich selbst um die Verwaltung des Bitmaps kümmert und es nur zur Verfügung stellt. Also mit anderen Worten beim Erstellen des StaticObject wird das Bitmap angelegt und evtl schon befüllt und beim Zeichnen greifst du nur noch auf dieses erstellte Bitmap zu.

Wenn das Bitmap sehr dynamisch sein muss, dann kannst es aber auch trotzdem so lösen. Aber in dem Appear dann einfach nur das interne Bitmap befüllen, zurückgeben und nur benutzen. Also nicht freigeben. Um es etwas zu verdeutlichen.

Delphi-Quellcode:

type
  TStaticObject = class
  private
    fBitmap: TBitmap;
  public
    property Bitmap: Integer read fBitmap;

    function Appear: TBitmap;

    constructor Create;
    destructor Destroy; override;
  end;


constructor TStaticObject.Create;
begin
  inherited;
  fBitmap := TBitmap.Create;
  // Entweder hier schon befüllen.
end;

destructor TStaticObject.Destroy;
begin
  fBitmap.Free;
  inherited;
end;

function TStaticObject.Appear: TBitmap;
begin
  // Oder hier das Bitmap befüllen und zurückgeben
  Result := fBitmap;
end;

Darstellen könntest du es dann folgendermaßen.

Delphi-Quellcode:

// entweder direkt das Bitmap darstellen
GameScreen.Canvas.Draw(StaticObject.PosX, StaticObject.PosY, StaticObject.Bitmap); 

// oder aber mit der Methode die nur das Bitmap zurück gibt.
GameScreen.Canvas.Draw(StaticObject.PosX, StaticObject.PosY, StaticObject.Appear);

Damit könntest du bei jedem Zeichnen das Bitmap anpassen (Methode Appear), falls das StaticObject doch mehr ein dynamisches Objekt ist. Du würdest aber a. nicht ständig bitmaps anlegen und freigeben und b. hättest auch kein Speicherloch und c. einfacher code und klare regelung wer sich um das Bitmap zu kümmern hat. Nämlich die Klasse TStaticObject zu der es ja schließlich gehört und niemand anders.

PS: Es ist übrigens nicht so abwägig, dass sich die Objekte selber zeichnen. Denn eigentlich ist das genau der tiefere Sinn hinter OOP. Die Szene muss so immer genau wissen wie die Objekte gezeichnet werden müssen. Und das gehört eigentlich nicht zu ihren Aufgaben. Besser (klarrer Strukturiert) wäre es, wenn nur das Objekt wüsste wie es gezeichnet werden muss und sich auch selbst darum kümmert. Die Szene würde dann nur sagen. ObjektXYZ zeichne dich mal. Das aber nur so am Rande. ;)

[Desmulator]

Also das Problem ist allerdings, dass das BitMap immer anders aussehen kann und man auch noch die Farben verstellen kann...
Ich zeige euch einfach mal die gesamte Klasse:

Delphi-Quellcode:

type
  TStaticObjectMouseEvent = procedure(X:integer;Y:integer; //MousePosition on the Object
                                      ScreenX:integer;ScreenY:integer); //MousePosition on the Screen

  TStaticObject = class(TObject)
    private
      { Grafik }
      Graphic : TBitMap; //Eine einfache BitMap

      { Events }
      FOnClick : TStaticObjectMouseEvent;   //When the User klicks on the Object
      FOnMouseOver : TStaticObjectMouseEvent;  //When the mouse goes over the Object
      FOnMouseOut : TStaticObjectMouseEvent;  //When the mouse goes out the Object

    public
      { Position & Größe , Read + Write }
      PosX  : integer;
      PosY  : integer;
      Width : integer;
      Height: integer;
      ZIndex: integer;

      { Farben }
      Colors : TRGBColorList;

      { Boolische Ausdrücke, Read + Write }
      AutoSize: boolean;
      //Ändert die Breite und Höhe automatisch auf orginal, ansonsten wir aus Width & Height zugeschnitten
      Visible : boolean;
      //Ist das Objekt sichbar oder nicht, wird von T2DScreen geprüft
      //Appear gibt ein leerBmp zurück


      { Sonstige Eigenschaften }
      TranzparentColor: TColor;
      Tranzparent : boolean;

      { Eigenschaften der Events }
      property OnClick : TStaticObjectMouseEvent read FOnClick write FOnClick;
      property OnMouseOver : TStaticObjectMouseEvent read FOnMouseOver write FOnMouseOver;
      property OnMouseOut : TStaticObjectMouseEvent read FOnMouseOut write FOnMouseOut;
      { TODO 3 : Die Funktionen für den Aufruf der Methode müssen noch geschrieben werden }


      { Bild }
      property Image : TBitMap read Graphic;

      { Constructor }
      constructor Create(FilePath: PAnsiChar;Visibility : boolean = false);
      destructor Destroy;

      { Proceduren }
      function ChangeGraphic(FilePath : PAnsiChar) : boolean;
      function Appear( const Dest: TBitmap ): TBitMap;


    end;



implementation


constructor TStaticObject.Create(FilePath: PAnsiChar; Visibility : boolean = false);
begin

  //Prüfen wir ob di Datei existiert
  if fileexists(FilePath) then begin
    try //Fehlern vorbeugen

      //Grafik laden
      ChangeGraphic(FilePath);

      //Boolische Tüpen setzen
      AutoSize := true;
      Visible := Visibility;

      //Colors
      Colors.Red := 255;
      Colors.Green := 255;
      Colors.Blue := 255;
      Colors.Alpha := 255;

    except
      //Selbst löschen
      self.Free;
    end;
  end;
end;


function TStaticObject.ChangeGraphic(FilePath: PAnsiChar) : boolean;
begin
  if fileexists(FilePath) then begin
    try //Fehlern vorbeugen

      //Prüfen ob Graphic eine Instanz ist
      if Assigned(Graphic) then
        Graphic.FreeImage;

      //Graphic TBitMap erstellen
      Graphic := TBitMap.Create;
      //BitMap laden
      Graphic.LoadFromFile(FilePath);

      //Rückgabe
      Result := true;

    except
      Result := false;
    end;
  end;
end;


destructor TStaticObject.Destroy;
begin
  try
    //Graphic freigeben
    Graphic.FreeImage;
  finally
    self.Free; //Selber freigeben
  end;
end;

function TStaticObject.Appear( const Dest: TBitmap ) : TBitMap;
begin
  try

    //Autosize
    if Autosize and Visible then begin
      Width := Graphic.Width;
      Height := Graphic.Height;
    end else if not Visible then begin
      Width := 0;
      Height := 0;
    end;

    //Imagegröße
    Dest.Width := Width;
    Dest.Height := Height;

    { DONE 1 : Die Eigenschaft Visible muss noch berücksichtigt werden }

    if Visible then begin

      //Kopiern und ggf. skalieren
      Dest.Canvas.StretchDraw(bounds(0,0,Width,Height),Graphic);

      //Bildtranzparents
      Dest.Transparent := Tranzparent;
      Dest.TransparentColor := TranzparentColor;
      Dest.TransparentMode := tmFixed;

      //Prüfen ob die Farberwerte geändert wurden
      if ( Colors.Red <> 255 ) or ( Colors.Green <> 255 ) or ( Colors.Blue <> 255 ) then
        FilterColors(Colors,Dest);
    end;

  finally
    //Nichts
  end;
end;

Das Problem an deinem Vorschlag ist, dass das Bild extrem dynamisch ist und sich in jedem Frame verändern kann. Ob größe oder Farbe und Tranzparentz... Sonst wäre das ja ganz einfach.
Und unter einem statischen Objekt verstehe ich ein Objekt das nicht animiert ist, also was aus keinem FarmeSet besteht.
Man kann zwar in jedem Frame das Bild wechseln, jedoh ist dafür das TAnimatedObject besser geeignet... 8)

Das Problem beim selber zeichnen ist, dass der Bildschrim auch events wie OnClick und OnMouseOver/Out bearbeiten muss...

[Lossy eX]

Zum Thema ändern: Ja wo ist da jetzt das Problem? Es geht mir ja nur darum, dass das Objekt sein Bitmap selber verwaltet und es außerhalb deines Objektes nur benutzt wird. Was und wann dieses Objekt mit dem Bitmap macht ist ihm überlassen. Beim Zeichnen kannst du ja trotzdem eine Funktion aufrufen die das interne Bitmap aktualisiert und diesen dann zurückgibt.

Da du bei dir das Bild noch in der Größe verändern möchtest benötigst du also 2 Bitmaps. Ein Mal das Originale und ein Mal das zu zeichnende Bild. Wobei es wohl besser wäre, wenn du das originale Bild gestreckt direkt auf das Canvas zeichnen würdest. Denn das würde zeichnerrei und Speicher sparen. Evtl wäre es langsamer als wie wenn man das zu zeichnende Bild zwischenspeichert aber da du das in deinem Code ja auch jedes mal neu erstellst wird es zu mindest nicht langsamer als der bestehende Code.

Hier noch ein paar Hinweise zu deinem Code:
Im Destruktor nicht Self.Free aufrufen sondern IMMER Inherited und die Methode auch IMMER überschreiben. Also override benutzen. Außer es hat einen sehr triftigen Grund. Und den sehe ich bei dir nicht. ;) Solltest du Destroy überschreiben und self.free aufrufen endet das in einer Endlosschleife. Wenn ich mich nicht vertue.

Ein Self.Free im Falle eines Fehlers innerhalb Konstruktors. Das kannst du dir sparen, denn sobald im Konstruktor ein Fehler auftritt der diesen beenden würde, wird die Instanz wieder freigegeben. Außerdem wird die Exception weiter gereicht. In deinem Falle würde die Exception unterdrückt werden aber die Instanz würde wieder freigegeben werden. Was evtl sogar fatal wäre, da es sogar so sein müsste/könnte, dass TStaticObject.Create einen Pointer zurück liefern müsste. Aber einen Pointer auf ein Objekt welches schon wieder freigegeben wurde. Du kannst als Später nicht mehr entscheiden ob es richtig ist oder nicht.

Graphic.FreeImage: Freeimage löscht nicht die Instanz sondern lediglich die Bilddaten. Die Instanz existiert also weiterhin. Das verringert zwar das Speicherloch erheblich. Aber es ist trotzdem eines da.

Auch wenn es ein bisschen Offtopic wird. ;) TAnimatedObject und TStaticObject sollten einen gemeinsamen Vorfahren haben. Beide von TObject abzuleiten ist kein schöner OOP Stil. Denn genau genommen unterscheiden sie sich "lediglich" dadurch, dass das TAnimatedObject eine Animation hat. Ansonsten sind es beide irgendwelche Dinger die gezeichnet werden. Und falls doch noch mehr Unterschiede existieren, dann kann man auch noch eine Ableitung dazwischen setzen.

Zum Thema OnClick. Ich weiß nicht was du da so machst aber eigentlich ist so etwas eben auch prädestiniert für OOP. Ich habe solche Konstruktionen schon ein paar mal gemacht und hat bisher immer geklappt. Evtl solltest du mal andenken in einem anderen Thema mal nach Designvorschlägen/Ideen/Anregungen fragen. So etwas macht im Endeffeckt dann auch deine Arbeit leichter. Speziell wenn es noch komplizierter wird. Ist aber nur eine Idee.

[Desmulator]

OKay danke für die Tipps, allerdings habe ich manches noch net so wirklich verstanden.. :roteyes:

Also das mit dem try...except self.free kann ich im constructor weg lassen, da die exception zurück gegeben wird.
Dann habe ich das nicht so verstanden was ich jetzt mit dem Destructor machen soll, also mit override überschreiben und dann? Was ist jetzt mit dem Inherited Free?

Sonst geht alles Thx
Edit: die tranzparents geht aus irgendeinem grund verlohren bei übergabe in das TempBMP?
Edit2: Hihi okay die Tranzparents is auch wieder da^^

[Lossy eX]

Den Destruktor wie in meinem anderen Post. Also override in der Klasse anhängen und in der implementation nur inherited; aufrufen.

Zur Erklärung. Der Destruktor ist virtuell. Deswegen muss man den Überschreiben, da sonst die Methode nur verdeckt wird. Und inherited dient dazu um eine Methode aus der Vorfahrenklasse aufzurufen. Damit auch andere Destruktoren aufgerufen werden. Free ruft intern auch nur Destroy auf. Plus eine Sicherheitsabfrage. Falls zu Klasse noch Lücken sind würde ich aber eher zu einem Tutorial raten ansonsten tut dir keinen Zwang an. ;)

[Muetze1]

Zitat:

Zitat von DeddyH

Zitat:

Zitat von Muetze1

Delphi-Quellcode:

function TStaticObject.Appear( const Dest: TBitmap ) : TBitMap;

Ich denke, eher so:

Delphi-Quellcode:

procedure TStaticObject.Appear( const Dest: TBitmap );

Der Rückgabewert hat sich somit erledigt, oder mache ich jetzt einen Denkfehler?

Ups, dass habe ich beim editieren vom Original noch übersehen. Natürlich eine ergebnislose Methode, somit muss das noch weg, richtig. Danke!