[2/2] gdiplus: Implement GdipBeginContainer2 and GdipEndContainer

[Nikolay Sivov]

Andrew Eikum wrote:
> Nikolay Sivov wrote:
>> Andrew Eikum wrote:
>>> ---
>>>  dlls/gdiplus/gdiplus_private.h |    3 +
>>>  dlls/gdiplus/graphics.c        |  154 
>>> ++++++++++++++++++++++++++++++++++++++--
>>>  include/gdiplusflat.h          |    1 +
>>>  3 files changed, 152 insertions(+), 6 deletions(-)
>>>   
>> This looks wrong for me cause I don't think you could pass container 
>> id created with one Graphics to
>> Graphics.EndContainer() of another. I've just checked it and it 
>> doesn't return any error code,
>> but generated container ids aren't the same when you call 
>> BeginContainer() for e.g. two objects
>> as sequent steps. It looks like on native these value are module 
>> unique, some kind of handle table needed.
>> This kills the first problem I described - passing strange value to 
>> another Graphics.
>>
>> Also note that Save()/Restore() functionality uses the same stack 
>> with container calls (at least msdn tells us that),
>> for that Save/Restore we already have a test:
>> ---
>> static void test_save_restore(void)
>> ....
>>    /* The same state value should never be returned twice. */
>>    todo_wine
>>        check_no_duplicates(state_log);
>> ....
>> ---
>>
>> It clearly says that no duplicates allowed, even after multiple 
>> object deletion - it checks for values returned
>> during the whole test.
>
> I agree that the behavior of the container IDs doesn't match native.  
> However, do we really care to duplicate that behavior?  I can't see 
> any documentation in MSDN that says container IDs must be unique 
> across either graphics objects or within a single graphics object.
It doesn't matter is it explicitly documented or not. We should care 
only about how it really works.
> I can't imagine any application depending on the uniqueness of 
> container IDs; if they try to pass a container for graphics1 to 
> graphics2, nothing happens, so there's no behavior to depend on.
If you create container c1 for graphics1 (0 numbered), then c2 for 
graphics2 (0 numbered too), then pop container for graphics2 with c1 value
(c1 == c2) you implementation will successfully restore graphics2 to 
previous state. This doesn't happen on native - it will return Ok 
status, but
status will be retained. This is a behavior we should depend on.
> Ditto for invalid or already-used containers.  The only scenario I can 
> think of where the uniqueness of container IDs would be a dependency 
> would be an application error calling EndContainer() with invalid 
> values.  I don't know if it's worth the effort of making a handle 
> table and whatnot to work around an application glitch that might not 
> even exist. 
What you mean it might not? If somebody (e.g. me) will write a test for 
that will it exist then?
>
> So, I would argue against the validity of the test you quote.  Why 
> should the same value not be returned twice?
Agreed, test isn't so obvious cause it shows that id value doesn't 
reoccur even after releasing containers.
But test passes well on Windows, and only this matters here.

To simplify this we possibly could not to reproduce this exactly, but 
only guaranty ID uniqueness for currently valid Graphics objects in
a process wide way.

After that we could consider making this test pass as a second step to 
get closer to native behavior.