[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: [PATCH 03/10 v4] gdiplus: Add functions for managing metafile objects id
From:       Piotr Caban <piotr.caban () gmail ! com>
Date:       2017-06-30 20:53:36
Message-ID: D2743225-DB23-47D4-8DE2-772803501A39 () gmail ! com
[Download RAW message or body]


> On 30 Jun 2017, at 22:24, Vincent Povirk <madewokherd@gmail.com> wrote:
> 
> This seems like overkill to me?
> 
> I don't see the need for per-object refcounting. Every drawing
> operation that uses an object is just going to end with a series of
> releases, and after it returns all refcounts should be 0.
> 
> +    /* TODO: object id should be released on object dispose */
> I don't think this approach is possible. It would require every object
> to be aware of which metafiles are using it, and every operation that
> modifies the object would have to release them. Bitmap objects can
> refer to an application buffer that could be modified without any call
> into gdiplus. (Or, Bitmaps could be modified by an ImageAttributes, in
> which case we can't reuse them except with an identical
> ImageAttributes.)
> 
> If we wanted to reuse Bitmaps, we'd have to compare the data. For
> other large types like Paths, maybe keeping an id to detect whether it
> changed would make sense. For types of bounded size, it probably would
> be easier to store a complete copy.
Currently the implementation only overwrites ids in the same way as native does. It's \
there to make sure we never run out of IDs. I think it's nice that the functions are \
failsafe. It's also closer to native behaviour.

I think that everything else can be figured out later. The comment is based on \
observation of native dll behaviour. I've checked what happens when new bitmaps are \
created and drawn to metafile in a loop. All of the bitmaps contained exactly the \
same data. In this case the IDs were not freed unless we run out of free IDs. After \
adding GdiDisposeImage call to the loop the IDs got released. I think it makes the \
comment valid even if we never implement this behaviour.

Thanks,
Piotr


[prev in list] [next in list] [prev in thread] [next in thread]