[ wxwindows-Patches-1877682 ] Inconsistent wxDC:SetClippingRegion()

[SourceForge.net]

Patches item #1877682, was opened at 2008-01-22 23:00
Message generated for change (Comment added) made by csomor
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=309863&aid=1877682&group_id=9863

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Other
Group: bug fix
Status: Pending
Resolution: None
Priority: 5
Private: No
Submitted By: AmiArt (amiart)
Assigned to: Nobody/Anonymous (nobody)
Summary: Inconsistent wxDC:SetClippingRegion()

Initial Comment:
The current implementation of SetClippingRegion(const wxRect& cliRect) converts the clipRect parameter from logical to device units but the overloaded version SetClippingRegion(const wxRegion& cliRegion) uses clipRegion directly without log->dev conversion.

So this code will set different clip region:

dc->SetDeviceOrigin(10, 10);

// (1)
wxRect clipRect(0,0,100,100);
dc->SetClippingRegion(clipRect);

// (2)
wxRegion clipRegion(clipRect);
dc->SetClippingRegion(clipRegion);

The attached patch changes SetClippingRegion(const wxRegion&) function so it performs correct conversion in wxGTK and wxMSW ports. The same changes must be done for wxMAC, wxX11 etc.

----------------------------------------------------------------------

>Comment By: Stefan Csomor (csomor)
Date: 2008-02-04 09:01

Message:
Logged In: YES 
user_id=81467
Originator: NO

the API shows the history, at the root the Win32 implementation where the
SelectClipRgn does use device coordinates and then I guess the overload
came, which suddenly used logical coordinates ... but that's they way it is
on all ports, so I guess apart from adding an important note, we shouldn't
silently change the meaning, we could have perhaps have an additional
boolean param indicating whether it's device coordinates in the wxRect
case, defaulting to false eg ..

Best,

Stefan Csomor

----------------------------------------------------------------------

Comment By: Vadim Zeitlin (vadz)
Date: 2008-02-03 15:05

Message:
Logged In: YES 
user_id=71618
Originator: NO

I'm not sure if the bug is in wxRegion or wxRect version to be honest.
Maybe we should interpret wxRect in device coordinates instead of
interpreting wxRegion in the logical ones, wouldn't this be more useful?
Robert, as our main region expert, do you have an opinion about this?

But if we do decide that the approach of your patch is correct, it should
definitely be done at wxDC::SetClippingRegion() level (in wx/dc.h) so that
it is done for all ports at once. Unless any of them already do this
translation but I don't think this is the case.

And in any case I don't think we should be changing this in 2.8 branch as
there can be code relying on the existing behaviour (and maybe correcting
for it by doing the conversion itself).

P.S. Assigning reports to me doesn't ensure that I look at them sooner,
rather it can prevent others from looking at them before I can.

----------------------------------------------------------------------

Comment By: AmiArt (amiart)
Date: 2008-01-22 23:02

Message:
Logged In: YES 
user_id=1691168
Originator: YES

File Added: dcclient.cpp.patch

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=309863&aid=1877682&group_id=9863