blit appears to be fixed in 2.5.1.0p8, but brush fill color is broken

wxPython Dev
1

2.5.1.0p8 appears to fix the blit slowdown problem!

However, either SetBrush is broken or the drawing mode for the fill color is
busted as the simple test below shows. The fill color always shows up as
white with 2.5.1.0p8; this wasn't broke with 2.5.1.0p7

from wxPython import wx

class MyApp(wx.wxApp):

    def OnInit(self):
        frame = wx.wxFrame(None, -1, "minimal", size=(200, 200))
        panel = wx.wxPanel(frame, -1)

        frame.Show(1)

        dc = wx.wxClientDC(panel)
        brush = dc.GetBrush()
        brush.SetColour('blue')
        dc.SetBrush(brush)
        dc.DrawRectangle(5, 10, 50, 100)

        self.SetTopWindow(frame)

        return 1

app = MyApp(0)
app.MainLoop()

ka

2

Kevin Altis wrote:

2.5.1.0p8 appears to fix the blit slowdown problem!

However, either SetBrush is broken or the drawing mode for the fill color is
busted as the simple test below shows. The fill color always shows up as
white with 2.5.1.0p8; this wasn't broke with 2.5.1.0p7

I think that the fact that this worked before was due to a bug that has since been fixed.

Let me explain some of the behind the scene stuff to help you understand. The various C++ GDI classes use reference counting, meaning that the attributes of (for example) the wxBrush class (colour, flags, MS handle, etc.) are actually held in another class that wxBrush just has a pointer too. Then when you return an instance of wxBrush or otherwise make a copy only the pointer is copied. If you then set an attribute on the copy then a new instance of the ref data is created for the copy at that point in time and the new attribute is set there. (This is called copy-on-write, and is a common design pattern in C++.)

Finally, the wxDC::SetBrush (and etc.) will check if the brush being set is equivalent to the bush already set, and if so it just returns immediatly and will avoid the overhead of creating a new GDI handle and etc. for it.

Now let's walk through your example:

> dc = wx.wxClientDC(panel) #1
> brush = dc.GetBrush() #2
> brush.SetColour('blue') #3
> dc.SetBrush(brush) #4

Line 2 will return a C++ reference to the wxBrush instance in the DC (not a copy pointing to the same refdata.) In line 3 you set an attribute of that internal refdata, since there is only the one C++ instance there is no copy-on-write happening. In line 4 the DC checks if the brush being passed in is equivallent to the one already set, since they are the same C++ instance then they are, and so it returns immediately without creating a new MSW handle for the brush and selecting it into the HDC.

IIRC, the bug that was fixed was in the operator== or related to it, and so previously the optimization test in SetBrush was always failing.

I think that I can change things such that methods like GetBrush will return a new instance (hence always force a C++ copy,) and since the internal data is refcounted that will not be much overhead. Then when you set attributes the copy-on-write will happen and you will not be touching the instance already held by the DC. I'll need to think about this a bit though to ensure that there are no adverse side effects...

···

--
Robin Dunn
Software Craftsman
http://wxPython.org Java give you jitters? Relax with wxPython!

3

Robin Dunn wrote:

Now let's walk through your example:

> dc = wx.wxClientDC(panel) #1
> brush = dc.GetBrush() #2
> brush.SetColour('blue') #3
> dc.SetBrush(brush) #4

Line 2 will return a C++ reference to the wxBrush instance in the DC (not a copy pointing to the same refdata.) In line 3 you set an attribute of that internal refdata, since there is only the one C++ instance there is no copy-on-write happening.

But that instance of a wxBrush should have beeen changed, shouldn't it?

In line 4 the DC checks if the brush being passed in is equivallent to the one already set, since they are the same C++ instance then they are, and so it returns immediately without creating a new MSW handle for the brush and selecting it into the HDC.

So this is a no-op. but why didn't the brush that was currently selected into the DC change? or is a copy made when the brush is selected?

Also, I remember some wierd stuff happening on GTK when you tried to chagne the properties of a wxFont, rather than creating a new one. Is that a similar issue?

I think that I can change things such that methods like GetBrush will return a new instance (hence always force a C++ copy,) and since the internal data is refcounted that will not be much overhead. Then when you set attributes the copy-on-write will happen and you will not be touching the instance already held by the DC. I'll need to think about this a bit though to ensure that there are no adverse side effects...

It sure seems like the current behaviour could have some wierd side effects...-Chris

···

--
Christopher Barker, Ph.D.
Oceanographer
                                         
NOAA/OR&R/HAZMAT (206) 526-6959 voice
7600 Sand Point Way NE (206) 526-6329 fax
Seattle, WA 98115 (206) 526-6317 main reception

Chris.Barker@noaa.gov

4

Okay, so what is the correct way to set the fill color today and other brush
related attributes while preserving attributes other than the one you want
to set?

        dc = wx.wxClientDC(panel)
        # this line does work, but would lose
        # any other brush attributes of the current DC
        #dc.SetBrush(wx.wxBrush('blue'))

        # this doesn't work
        brush = dc.GetBrush()
        brush.SetColour('blue')

        # this doesn't work for the reasons Robin described
        #dc.SetBrush(brush)

        # this line doesn't work either
        #dc.GetBrush().SetColour('blue')

        dc.DrawRectangle(5, 10, 50, 100)

I don't want the returned value of GetBrush to deviate from how wxPython
handles other objects, especially since 2.5 is supposed to break backwards
compatability if it will make the API more consistent and fix past warts. I
would rather just get my act together and correct code where required. There
should be a note added to the 2.5 upgrade notes for issues like this since I
bet it is going to burn a lot of folks.

The whole brush/pen thing has always been one of those weird wxWindows API
warts that seemed obtuse to me since you to do a lot of code for the simple
things like setting a drawing color. Hmm, I wonder if there are similiar
wxPen issues I need to look at?!

Chris are you testing any of this stuff?

On a somewhat related note, I always use GetSizeTuple and GetPositionTuple
methods, convert the tuple to a list if needed, do my changes, and then call
SetSize or SetPosition because I used to run into weird bugs when
manipulating the lists returned by GetSize and GetPosition. I assume that
was a related reference issue that might have changed in 2.5?

Stupid-ly, yours,

ka
p.s. It is a good thing that wxPython book didn't get written based on the
2.4.x wxPython :wink:

···

-----Original Message-----
From: Robin Dunn [mailto:robin@alldunn.com]
Sent: Monday, March 01, 2004 12:12 PM
To: wxPython-dev@lists.wxwindows.org
Subject: Re: [wxPython-dev] blit appears to be fixed in 2.5.1.0p8, but
brush fill color is broken

Kevin Altis wrote:
> 2.5.1.0p8 appears to fix the blit slowdown problem!
>
> However, either SetBrush is broken or the drawing mode for the
fill color is
> busted as the simple test below shows. The fill color always shows up as
> white with 2.5.1.0p8; this wasn't broke with 2.5.1.0p7
>
>

I think that the fact that this worked before was due to a bug that has
since been fixed.

Let me explain some of the behind the scene stuff to help you
understand. The various C++ GDI classes use reference counting, meaning
that the attributes of (for example) the wxBrush class (colour, flags,
MS handle, etc.) are actually held in another class that wxBrush just
has a pointer too. Then when you return an instance of wxBrush or
otherwise make a copy only the pointer is copied. If you then set an
attribute on the copy then a new instance of the ref data is created for
the copy at that point in time and the new attribute is set there.
(This is called copy-on-write, and is a common design pattern in C++.)

Finally, the wxDC::SetBrush (and etc.) will check if the brush being set
is equivalent to the bush already set, and if so it just returns
immediatly and will avoid the overhead of creating a new GDI handle and
etc. for it.

Now let's walk through your example:

> dc = wx.wxClientDC(panel) #1
> brush = dc.GetBrush() #2
> brush.SetColour('blue') #3
> dc.SetBrush(brush) #4

Line 2 will return a C++ reference to the wxBrush instance in the DC
(not a copy pointing to the same refdata.) In line 3 you set an
attribute of that internal refdata, since there is only the one C++
instance there is no copy-on-write happening. In line 4 the DC checks
if the brush being passed in is equivallent to the one already set,
since they are the same C++ instance then they are, and so it returns
immediately without creating a new MSW handle for the brush and
selecting it into the HDC.

IIRC, the bug that was fixed was in the operator== or related to it, and
so previously the optimization test in SetBrush was always failing.

I think that I can change things such that methods like GetBrush will
return a new instance (hence always force a C++ copy,) and since the
internal data is refcounted that will not be much overhead. Then when
you set attributes the copy-on-write will happen and you will not be
touching the instance already held by the DC. I'll need to think about
this a bit though to ensure that there are no adverse side effects...

--
Robin Dunn
Software Craftsman
http://wxPython.org Java give you jitters? Relax with wxPython!

5

Chris Barker wrote:

Robin Dunn wrote:

Now let's walk through your example:

> dc = wx.wxClientDC(panel) #1
> brush = dc.GetBrush() #2
> brush.SetColour('blue') #3
> dc.SetBrush(brush) #4

Line 2 will return a C++ reference to the wxBrush instance in the DC (not a copy pointing to the same refdata.) In line 3 you set an attribute of that internal refdata, since there is only the one C++ instance there is no copy-on-write happening.

But that instance of a wxBrush should have beeen changed, shouldn't it?

Yes.

In line 4 the DC checks if the brush being passed in is equivallent to the one already set, since they are the same C++ instance then they are, and so it returns immediately without creating a new MSW handle for the brush and selecting it into the HDC.

So this is a no-op. but why didn't the brush that was currently selected into the DC change? or is a copy made when the brush is selected?

Because wxDC assumes that since m_brush==brush that a new Windows HBRUSH does not need to be created and selected into the HDC.

Also, I remember some wierd stuff happening on GTK when you tried to chagne the properties of a wxFont, rather than creating a new one. Is that a similar issue?

Yes.

···

--
Robin Dunn
Software Craftsman
http://wxPython.org Java give you jitters? Relax with wxPython!

6

Kevin Altis wrote:

Okay, so what is the correct way to set the fill color today and other brush
related attributes while preserving attributes other than the one you want
to set?

To change just the colour in p8 this should work:

  ob = dc.GetBrush()
  nb = wx.Brush(newColour, ob.GetStyle())
  s = ob.GetStiple()
  if s.Ok():
    nb.SetStiple(s)
  dc.SetBrush(nb)

I don't want the returned value of GetBrush to deviate from how wxPython
handles other objects, especially since 2.5 is supposed to break backwards
compatability if it will make the API more consistent and fix past warts.

Yes. I would have to make a similar change everywhere that the GDI objects are returned. I'm also considering how things are typically done from C++ to formulate a good set of use cases to help me decide...

I

would rather just get my act together and correct code where required. There
should be a note added to the 2.5 upgrade notes for issues like this since I
bet it is going to burn a lot of folks.

The whole brush/pen thing has always been one of those weird wxWindows API
warts that seemed obtuse to me since you to do a lot of code for the simple
things like setting a drawing color. Hmm, I wonder if there are similiar
wxPen issues I need to look at?!

Yep, wxPen will have the same behaviour.

On a somewhat related note, I always use GetSizeTuple and GetPositionTuple
methods, convert the tuple to a list if needed, do my changes, and then call
SetSize or SetPosition because I used to run into weird bugs when
manipulating the lists returned by GetSize and GetPosition. I assume that
was a related reference issue that might have changed in 2.5?

Take a look at the python class for wx.Point, wx.Size, etc. I've tried to make them a little more sane, and with SWIG 1.3 enabling properties then accessing the attributes is a lot cleaner and more efficient too. Each of them can be used as a fixed length sequence, so you can go things like this for tuple-unpacking:

  x,y = self.GetPosition()

or use indexing:

  pos = self.GetPosition()
  x = pos[0]; y = pos[1]

Also, if you really need to have the values as a tuple then all these classes now have a Get method, so you can do this:

  pos = self.GetPosition().Get()

So the Get*Tuple methods are really redundant at the moment, but I've left them in because they don't really cost anything other than the size of the code for the wrapper function. I've considered replacing them with small python wrappers that just call self.GetWhatever().Get() but havn't done it yet.

Stupid-ly, yours,

ka
p.s. It is a good thing that wxPython book didn't get written based on the
2.4.x wxPython :wink:

Yeah, we knew changes were coming so we started from the begining doing a mental "from __wx__future__ import *" :wink:

···

--
Robin Dunn
Software Craftsman
http://wxPython.org Java give you jitters? Relax with wxPython!

7

Oh for the love of Power Pete, that's crazy talk. Its this kind of thing that makes me think wxDC needs to have some methods added to directly change brush and pen attributes. Meanwhile, I'll go add some wrapper code.

ka

···

On Mar 1, 2004, at 1:26 PM, Robin Dunn wrote:

Kevin Altis wrote:

Okay, so what is the correct way to set the fill color today and other brush
related attributes while preserving attributes other than the one you want
to set?

To change just the colour in p8 this should work:

  ob = dc.GetBrush()
  nb = wx.Brush(newColour, ob.GetStyle())
  s = ob.GetStiple()
  if s.Ok():
    nb.SetStiple(s)
  dc.SetBrush(nb)

8

From: Robin Dunn

To change just the colour in p8 this should work:

  ob = dc.GetBrush()
  nb = wx.Brush(newColour, ob.GetStyle())
  s = ob.GetStiple()
  if s.Ok():
    nb.SetStiple(s)
  dc.SetBrush(nb)

Shame on you and me for not testing before posting. I think an extra check
is required to make sure the DC actually has a brush, unless that is simply
another bug and the DC should always have a brush?!

    nb = wx.wxBrush(newColour)
    ob = dc.GetBrush()
    if ob.Ok():
        nb.SetStyle(ob.GetStyle())
        s = ob.GetStiple()
        if s.Ok():
            nb.SetStiple(s)
    dc.SetBrush(nb)

When I didn't check the brush first I was getting the following error

  File "C:\Python23\Lib\site-packages\wx\gdi.py", line 426, in GetStyle
    return _gdi.Brush_GetStyle(*args, **kwargs)
wx.core.PyAssertionError: C++ assertion "wxAssertFailure" failed in
..\..\src\ms
w\brush.cpp(290): invalid brush

The code above appears to work in 2.4.x and 2.5.1.0p8

ka

9

Kevin Altis wrote:

From: Robin Dunn

To change just the colour in p8 this should work:

ob = dc.GetBrush()
nb = wx.Brush(newColour, ob.GetStyle())
s = ob.GetStiple()
if s.Ok():
  nb.SetStiple(s)
dc.SetBrush(nb)

Shame on you and me for not testing before posting. I think an extra check
is required to make sure the DC actually has a brush, unless that is simply
another bug and the DC should always have a brush?!

Oops, I was going to mention that in my first reply about this, but decided that it would just complicate the already complex explaination so I took it out. DCs do always have a brush, it is just not valid to begin with. If you draw with an invalid brush then it defaults to solid white (and black for the Pens).

···

--
Robin Dunn
Software Craftsman
http://wxPython.org Java give you jitters? Relax with wxPython!

10

From: Kevin Altis
        s = ob.GetStiple()
        if s.Ok():
            nb.SetStiple(s)

Sigh. GetStipple and SetStipple. Obviously I need to step away from the
machine for a while.

Sorry,

ka

11

Robin Dunn wrote:

I don't want the returned value of GetBrush to deviate from how wxPython
handles other objects, especially since 2.5 is supposed to break backwards
compatability if it will make the API more consistent and fix past warts.

Yes. I would have to make a similar change everywhere that the GDI objects are returned. I'm also considering how things are typically done from C++ to formulate a good set of use cases to help me decide...

For what it's worth, I was helping a co-worker with a wxWidgets C++ app, and we had a bug when trying to change the active Pen width. It worked on some platforms, but not others. The solution was to create a new wxPen, rather than change the exisiting one. So C++ has exactly the same problem.

Frankly, it seems to me that the problem is poor design in the C++ code. A whole wxPen should be copy-on-write, so that when you change it's attributes, it is a new wxPen, and will be treated as such. If you can't changee it there, you might as well change it at the Python level, as you propose. Maybe add a copy() method that returns a copy of a given GDI object.

What would happen if you do this:

a = wx.Pen("blue")
b = a
DC.SetPen(b)
a.SetColor("red")

# The DC still has a blue Pen

DC.SetPen(b)

# is the DCs pen still blue?

DC.SetPen(a)
# what about now?

Kevin Altis wrote:

Okay, so what is the correct way to set the fill color today and other brush
related attributes while preserving attributes other than the one you want
to set?

I think it's time for a CopyPen, etc set pf functions, ig Robin doesn't add them to wxPython. Maybe a method like:

class wxBrush:

     def Copy(self, color = None, style = None, stipple = None)
  b = wxBrush(self) # does this work?
  
         if color: b.SetColor(color)
  if style: b.SetStyle(style)
  if stipple: b.SetStipple(stipple)
  
         return b

Chris are you testing any of this stuff?

Not yet but testing stuff on 2.5 is on my short list now. However, I don't tend to try to re-use the Brush, etc. on a DC anyway, I've tended to structure my code so that I make sure to draw what I want, regardless of what was drawn before, I don't use make use of the DC to keep state for me, even though it is. I've also got bitten by this a couple times, so I'm even less likely to do it.

-Chris

···

--
Christopher Barker, Ph.D.
Oceanographer
                                         
NOAA/OR&R/HAZMAT (206) 526-6959 voice
7600 Sand Point Way NE (206) 526-6329 fax
Seattle, WA 98115 (206) 526-6317 main reception

Chris.Barker@noaa.gov

12

Chris Barker wrote:

Robin Dunn wrote:

I don't want the returned value of GetBrush to deviate from how wxPython
handles other objects, especially since 2.5 is supposed to break backwards
compatability if it will make the API more consistent and fix past warts.

Yes. I would have to make a similar change everywhere that the GDI objects are returned. I'm also considering how things are typically done from C++ to formulate a good set of use cases to help me decide...

For what it's worth, I was helping a co-worker with a wxWidgets C++ app, and we had a bug when trying to change the active Pen width. It worked on some platforms, but not others. The solution was to create a new wxPen, rather than change the exisiting one. So C++ has exactly the same problem.

It depends on how you do it, if you use the reference directly then it will, but if you use the reference to make a copy then it won't. For example, with line 1 you will end up with a reference to the same instance used in the DC (just like it currently working in wxPython p8) but with line 2 you'll get a copy of the wxPen which will then do copy-on-write when you set an attribute:

  wxPen& p = dc.GetPen();

  wxPen p = dc.GetPen();

Frankly, it seems to me that the problem is poor design in the C++ code. A whole wxPen should be copy-on-write, so that when you change it's attributes, it is a new wxPen, and will be treated as such. If you can't changee it there, you might as well change it at the Python level, as you propose. Maybe add a copy() method that returns a copy of a given GDI object.

What would happen if you do this:

a = wx.Pen("blue")
b = a

Both a and b are Python references to the same wx.Pen proxy object, wich references a single wxPen C++ object.

DC.SetPen(b)
a.SetColor("red")

# The DC still has a blue Pen

Yes. Because the DC hasn't recreated the native Pen yet.

DC.SetPen(b)

# is the DCs pen still blue?

In this case, No. Because you started out with a new Pen and the first SetPen makes a copy of it. If you started out with a reference to an existing Pen already selected into the DC like in Kevin's example then this SetPen compares two references to the same instance and then would do nothing because they are identical.

DC.SetPen(a)
# what about now?

a and b are still the same Python object, and so they have the same attributes.

BTW, it looks like I can create a typemap that will change the code generated for handling the return of a wxBrush, etc. reference to be like the 2nd line of the C++ example above, i.e., it will create a copy to return, instead of the original reference, so then the copy-on-write will work as expected. By using a typemap I won't have to hunt down and evaluate all the places that it should be used, SWIG will do it for me. Now I just need to figure out which return types to apply the typemap to. wxBrush, wxPen, and wxFont are obvious, but what about wxBitmap, wxIcon and wxCursor? Is there any place that you would do a GetBitmap and expect to be able to operate on the original bitmap without having to do another SetBitmap later?

···

--
Robin Dunn
Software Craftsman
http://wxPython.org Java give you jitters? Relax with wxPython!

13

In this case, No. Because you started out with a new Pen and the first SetPen makes a copy of it. If you started out with a reference to an existing Pen already selected into the DC like in Kevin's example then this SetPen compares two references to the same instance and then would do nothing because they are identical.

<snip>

BTW, it looks like I can create a typemap that will change the code generated for handling the return of a wxBrush, etc. reference to be like the 2nd line of the C++ example above, i.e., it will create a copy to return, instead of the original reference, so then the copy-on-write will work as expected. By using a typemap I won't have to hunt down and evaluate all the places that it should be used, SWIG will do it for me. Now I just need to figure out which return types to apply the typemap to. wxBrush, wxPen, and wxFont are obvious, but what about wxBitmap, wxIcon and wxCursor? Is there any place that you would do a GetBitmap and expect to be able to operate on the original bitmap without having to do another SetBitmap later?

This whole issue probably needs more thought before a "solution" is implemented. Off the top of my head...
1. Does it only apply to classes in wxWindows that use reference counting?
2. There are instances where calling GetBrush, GetPen, etc. might return an invalid object, so at least in some cases you have to create a new object and follow-up with SetBrush, SetPen anyway.
3. How does wxDC::SetOptimization impact any solution? Having to special-case code for each platform in our Python apps is exactly the kind of thing we don't want, wxWindows or wxPython should be doing the work not every app programmer. Maybe SetOptimization should be off by default?!
4. Is using Get and then changing in-place more Pythonic?
5. Maybe SetBrush, SetPen... shouldn't be checking pointers as the test for equivalency? Is this in essence what SetOptimization is about? What is the real performance penalty if you set the brush or pen and the values are the same?!
6. The code example of just changing one attribute like the color of a brush definitely needs to be simpler.
7. I still think we should consider having additional direct wxDC methods for changing the pen color, brush color, etc.
8. Are we going to get into trouble by using up scarce GDI resources on Win9x?

ka

···

On Mar 1, 2004, at 3:49 PM, Robin Dunn wrote:

14

Robin Dunn wrote:

Now I just need to figure out which return types to apply the typemap to. wxBrush, wxPen, and wxFont are obvious, but what about wxBitmap, wxIcon and wxCursor? Is there any place that you would do a GetBitmap and expect to be able to operate on the original bitmap without having to do another SetBitmap later?

I can't imagine where, but would that work anyway? I think that's why the current setup with Pens and Brushes is so confusing... if I get a reference to the Pen with:

p = DC.GetPen

then I should be able to change it:

p.SetColor("blue")

and not even call:

DC.SetPen(p)

If the Pen had changed in place. What's really happening is that the Pen has changed in place, but the DC is not drawing with that wxPEn, it's drawing with a native pen that was copied at some point form teh wxPen, and won't get re-copied from p if p is refernceing is the same wxPen.

with a wxBitmap, I'd want to be able to do:

Button = wxBitmapButton(WHatever....)
.
bm = Button.GetBitmap()

MemDC.SelectObject(bm)
MemDC.DrawSomething()
del MemDC

# now, has the Bitmap on the button changed???

Button.SetBitmap(bm)

# It certainly should have now!

I think I get this now, but I'm still confused about what seems like strange logic in the C++ objects. It's trying to optimize behind the scenes, by using reference counting and copy-on-write for wxPens (etc.). However, when you write to a wxPen with SetColor(...) you don't get a copy then. wierd. Also:

> It depends on how you do it, if you use the reference directly then it
> will, but if you use the reference to make a copy then it won't. For
> example, with line 1 you will end up with a reference to the same
> instance used in the DC (just like it currently working in wxPython
> p8) but with line 2 you'll get a copy of the wxPen which will then do
> copy-on-write when you set an attribute:

> wxPen& p = dc.GetPen();

> wxPen p = dc.GetPen();

This shows how much (little) I know about C++. From the docs,

wxBrush& GetBrush()

I don't see it overloaded for the second form...can you even do that?

-Chris

···

--
Christopher Barker, Ph.D.
Oceanographer
                                         
NOAA/OR&R/HAZMAT (206) 526-6959 voice
7600 Sand Point Way NE (206) 526-6329 fax
Seattle, WA 98115 (206) 526-6317 main reception

Chris.Barker@noaa.gov

15

Kevin Altis wrote:

BTW, it looks like I can create a typemap that will change the code generated for handling the return of a wxBrush, etc. reference to be like the 2nd line of the C++ example above, i.e., it will create a copy to return, instead of the original reference, so then the copy-on-write will work as expected. By using a typemap I won't have to hunt down and evaluate all the places that it should be used, SWIG will do it for me. Now I just need to figure out which return types to apply the typemap to. wxBrush, wxPen, and wxFont are obvious, but what about wxBitmap, wxIcon and wxCursor? Is there any place that you would do a GetBitmap and expect to be able to operate on the original bitmap without having to do another SetBitmap later?

This whole issue probably needs more thought before a "solution" is implemented. Off the top of my head...
1. Does it only apply to classes in wxWindows that use reference counting?

The typemap can be applied to any of the following: wxBrush, wxPen, wxFont, wxBitmap, wxIcon, wxCursor, wxRegion.

2. There are instances where calling GetBrush, GetPen, etc. might return an invalid object, so at least in some cases you have to create a new object and follow-up with SetBrush, SetPen anyway.

Returning a copy will still show up as invalid (brush.Ok() returns false) but as soon as you set an attribute the copy-on-write kicks in and creates a wxBrushRefData instance for it and it then becomes valid.

3. How does wxDC::SetOptimization impact any solution? Having to special-case code for each platform in our Python apps is exactly the kind of thing we don't want, wxWindows or wxPython should be doing the work not every app programmer. Maybe SetOptimization should be off by default?!

Actually, it appears that none of the wx ports currently do anything for SetOptimization.

4. Is using Get and then changing in-place more Pythonic?

Probably, but it can't be done without recreating the native pen/brush/font (or at least tracking a 'dirty' flag) before each dc.Draw* method call. And that is overhead that the C++ folks would find unacceptable.

5. Maybe SetBrush, SetPen... shouldn't be checking pointers as the test for equivalency?

It's not. The == operator looks like this:

bool wxBrushRefData::operator==(const wxBrushRefData& data) const
{
     // don't compare HBRUSHes
     return m_style == data.m_style &&
            m_colour == data.m_colour &&
            m_stipple == data.m_stipple;
}

but since your original example ended up calling SetBrush with the same instance as was already set then the above test always returns true.

Is this in essence what SetOptimization is about?

No, I think it is supposed to be a hook for doing platform specific stuff, lower level things than wxPen, etc.

What is the real performance penalty if you set the brush or pen and the values are the same?!

A new native object is created, then a native API is called to select it into the DC and another will be called to release the old one (if it's refcount has dropped to zero), plus a bit of wx-specific overhead. So there are at least three native APIs called, plus some allocations and deallocations.

6. The code example of just changing one attribute like the color of a brush definitely needs to be simpler.

With my typemap change it will be just like you had in your sample:

  brush = dc.GetBrush()
  brush.SetColour("blue")
  dc.SetBrush(brush)

7. I still think we should consider having additional direct wxDC methods for changing the pen color, brush color, etc.

What do you mean?

8. Are we going to get into trouble by using up scarce GDI resources on Win9x?

No, that's what the C++ ref-counting/copy-on-write design is intended to help with. You only get a new native GDI object when you create a new one from scratch, or when you change the attribute of an existing one via a second (C++) reference to it.

···

--
Robin Dunn
Software Craftsman
http://wxPython.org Java give you jitters? Relax with wxPython!

16

Chris Barker wrote:

Robin Dunn wrote:

Now I just need to figure out which return types to apply the typemap to. wxBrush, wxPen, and wxFont are obvious, but what about wxBitmap, wxIcon and wxCursor? Is there any place that you would do a GetBitmap and expect to be able to operate on the original bitmap without having to do another SetBitmap later?

I can't imagine where, but would that work anyway? I think that's why the current setup with Pens and Brushes is so confusing... if I get a reference to the Pen with:

p = DC.GetPen

then I should be able to change it:

p.SetColor("blue")

and not even call:

DC.SetPen(p)

If the Pen had changed in place. What's really happening is that the Pen has changed in place, but the DC is not drawing with that wxPEn, it's drawing with a native pen that was copied at some point form teh wxPen, and won't get re-copied from p if p is refernceing is the same wxPen.

Yep.

with a wxBitmap, I'd want to be able to do:

Button = wxBitmapButton(WHatever....)
.
bm = Button.GetBitmap()

MemDC.SelectObject(bm)
MemDC.DrawSomething()
del MemDC

# now, has the Bitmap on the button changed???

It will be after a Refresh.

It turns out that the wxBitmapButton::Get*Bitmap methods are already returning copies of the wxBitmap (but pointing to the same internal ref-counted object.) Also, since drawing on the bitmap implicitly works on the internal native bitmap data {far below the wx level) your example here will work as you expect. It's only if you were to change the higher level attributes like width, or height, or load a new file into the wxBitmap that the copy-on-write functionality will kick in.

> It depends on how you do it, if you use the reference directly then it
> will, but if you use the reference to make a copy then it won't. For
> example, with line 1 you will end up with a reference to the same
> instance used in the DC (just like it currently working in wxPython
> p8) but with line 2 you'll get a copy of the wxPen which will then do
> copy-on-write when you set an attribute:

> wxPen& p = dc.GetPen();

> wxPen p = dc.GetPen();

This shows how much (little) I know about C++. From the docs,

wxBrush& GetBrush()

I don't see it overloaded for the second form...can you even do that?

It's part of the C++ language. What happens in the second case is that the reference returned by GetPen is implicitly used as a parameter to the wxPen copy constructor. It's the same as:

  wxPen p(dc.GetPen());

If p had already existed then the assignment operator would have been called instead. So this:

  wxPen p;
  p = dc.GetPen();

is the same as this:

  wxPen p; // default ctor is called
  p.operator=(dc.GetPen());

Python doesn't have (or need [usually]) implicit copy constructors or assignment operartors.

···

--
Robin Dunn
Software Craftsman
http://wxPython.org Java give you jitters? Relax with wxPython!