Bug 5676 - CAIROMM: Add bindings for more surface types
Summary: CAIROMM: Add bindings for more surface types
Status: RESOLVED FIXED
Alias: None
Product: cairo
Classification: Unclassified
Component: general (show other bugs)
Version: 1.1.1
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Carl Worth
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-01-21 10:37 UTC by Jonathon Jongsma
Modified: 2006-01-21 08:14 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch to add new Surface Types (53.14 KB, patch)
2006-01-21 10:44 UTC, Jonathon Jongsma
Details | Splinter Review
Revised patch with changes noted above (49.12 KB, patch)
2006-01-22 03:00 UTC, Jonathon Jongsma
Details | Splinter Review

Description Jonathon Jongsma 2006-01-21 10:37:02 UTC
I'm attaching a patch which adds C++ bindings for more of the Surface types. 
They're implemented in a basic flat heirarchy, i.e.:

Surface
  - ImageSurface
  - XlibSurface
  - Win32Surface
  - PdfSurface
  - PsSurface
  - SvgSurface
  - GlitzSurface
Comment 1 Jonathon Jongsma 2006-01-21 10:44:08 UTC
Created attachment 4413 [details] [review]
Patch to add new Surface Types

A couple things to note about this patch.  I added exception specifications for
all of the Surface API functions (i.e. functionname() throw(std::bad_alloc);). 
I don't know if this is controversial or not, but it seems like the right thing
to do to me.  Perhaps they should just specify the generic 'throw()' without
specifying which exceptions can be thrown?  I can rework it if people think
it's unnecessary.  

Also, I removed the status check from a couple of functions that (according to
my perusal of the cairo source code) don't seem to change the internal status
of the cairo object.

All Surface functions and classes have API documentation in doxygen.
Comment 2 Murray Cumming 2006-01-21 23:17:23 UTC
About exceptions:
We have generally avoided this in gtkmm and libxml++ because it seems to make
life more difficult and it requires us to know about every possible exception at
the time that we stabilize the API/ABI. Sorry, but I don't remember all of the
reasoning. The best I can find at the moment is here:
http://marc.theaimsgroup.com/?l=gtkmm&m=101515928306805&w=2
So, for now, I'd like to remove the throw() specifications.

Doxygen has @throws which does the job of documenting it without making extra
demands on the programmer.

>  I removed the status check from a couple of functions that (according to
> my perusal of the cairo source code) don't seem to change the internal status
> of the cairo object.

Is there any way to be sure that these won't change something in future? I'd
like some documentation/confirmation of this from the cairo developers. So far,
Carl Worth has told us that it's probably a good idea to check the status after
every call, in languages where that is viable.

I don't think that get_user_data() should be wrapped at all. It doesn't seem
useful. I think I have documented that elsewhere. If we add it now, even
protected, we can't remove it later.

What does "XXX" mean in comments? Please be more explicit. I use TODO: for
things that need more attention.

Otherwise this is wonderful and should be committed.
Comment 3 Jonathon Jongsma 2006-01-22 01:47:34 UTC
(In reply to comment #2)
> http://marc.theaimsgroup.com/?l=gtkmm&m=101515928306805&w=2
> So, for now, I'd like to remove the throw() specifications.

That sounds reasonable, I'll rework it to remove the exception specifications.

> Carl Worth has told us that it's probably a good idea to check the status after
> every call, in languages where that is viable.

ok.  Then I can easily put them back in.  There should be a fairly negligible
performance impact in any case since they're inlined, and there are only a
couple functions where it even applies.  

> I don't think that get_user_data() should be wrapped at all. It doesn't seem
> useful. I think I have documented that elsewhere. If we add it now, even
> protected, we can't remove it later.

I agree that it's not particularly useful at this point, but I didn't add these
functions.  They were previously wrapped in the public API, I just moved them to
protected.  Should I remove them altogether?  What are your expectations for API
stability for cairomm at this point?  
Comment 4 Murray Cumming 2006-01-22 02:12:46 UTC
> They were previously wrapped in the public API, I just moved them to
> protected.  Should I remove them altogether? 

Yes, I think that would be best. Thanks.

>  What are your expectations for API stability for cairomm at this point?  

Let's try not to make unnecessary breaks, particularly when we know that an
application is using the API. But I don't plan to officially freeze the API
until gtkmm 2.9/2.10 freezes it's API, around the time that GTK+ does, around
May 2006:
http://www.gtk.org/plan/2.10/

Only at that time are people likely to really look at the API and tell us about
problems.
Comment 5 Jonathon Jongsma 2006-01-22 03:00:08 UTC
Created attachment 4421 [details] [review]
Revised patch with changes noted above

- removed exception specifications
- removed *_user_data() functions
- added back the check_status_and_throw_exception() functions for functions
where it was commented out

I still haven't heard back about my CVS account request, so I'm not able to
commit this myself yet.
Comment 6 Murray Cumming 2006-01-22 03:14:47 UTC
Committed. Well done.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.