| Summary: | 'cairo_restore()' doesn't restore context in error case | ||
|---|---|---|---|
| Product: | cairo | Reporter: | Iosif Haidu <iosif.haidu> |
| Component: | win32 backend | Assignee: | cairo-bugs mailing list <cairo-bugs> |
| Status: | RESOLVED WONTFIX | QA Contact: | cairo-bugs mailing list <cairo-bugs> |
| Severity: | enhancement | ||
| Priority: | medium | ||
| Version: | 1.5.8 | ||
| Hardware: | x86 (IA32) | ||
| OS: | Windows (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
If those errors are not fatal, then cairo should be fixed. What you propose however is against the design of cairo. Moreover, when an object goes into an error state, all bets are off, the internal state of the object may not be consistent anymore. It's hard to return it back to a sane state. (In reply to comment #1) > If those errors are not fatal, then cairo should be fixed. What you propose > however is against the design of cairo. Moreover, when an object goes into an > error state, all bets are off, the internal state of the object may not be > consistent anymore. It's hard to return it back to a sane state. Well, actually the internal state of the cairo device context has nothing wrong after one of the windows gdi functions returns with error. But let's analyse for a while the meaning of those two cairo functions: 'cairo_save()' and 'cairo_restore()'. The first one will save a given cairo context on an internal stack while the second one restore it. Now, shouldn't these two functions provide a transaction like functionality ? And we know that the final result of any transaction is to return to a stable state (even if commit or rollback has been issued). But this doesn't happen with the 'cairo_restore()' function. It doesn't restore (rollback) the previous state if something wrong happens between. And it shouldn't be like that. If I would save the state of a variable on an internal stack I expect also to restore it if something goes wrong between (otherwise what's the point of those two functions ?). Also the cairo documentation doesn't specifiy that 'cairo_restore()' will not restore the previous cairo context if an error occurs between 'cairo_save()' and 'cairo_restore()'. Regarding clearing the error code in 'cairo_restore()': it's not necessary to propagate the error code generated between 'cairo_save()' and 'cairo_restore()' after the execution of 'cairo_restore()', because the developer can check in any point for an error situation between these two functions. The 'cairo_restore()' should set the error code only if it's not able to restore the previous context. If in some cases is necessary for 'cairo_restore()' to indicate that an error occured between, then probably it shold return that error code as result but internally should reset it, otherwise next time a new 'cairo_save()' - 'cairo_rstore()' block is executed, it will exit without executing anything. You could also let the developer to reset the error code, but then you must provide a way to do it (currently there is no access to cairo context status field and there is no method to reset the error code). Recreating the cairo context in an error situation could be a solution but it's not elegant. Why should I recreate a cairo context if there is nothing wrong with it ? Best regards, Iosif Haidu (In reply to comment #2) > But let's analyse for a while the meaning of those two cairo functions: > 'cairo_save()' and 'cairo_restore()'. You go on to discuss API semantics, (presumably proposing new semantics). If you're serious about this, the correct place to have the discussion is the cairo mailing list, not a bugzilla entry. We don't make API additions/changes without discussion on that list. See here for subscription information: http://cairographics.org/contact -Carl An object should only be put into an error condition if we detect an unrecoverable error. These semantics work very well with the batch rendering design of the API. If you feel this is inadequate and can propose a more advantageous design, then please open a discussion on the mailing list. |
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.
Hi, I would like to proprose to include in the next versions a mechanism that will allow to specify if 'cairo_restore()' function should restore the previously saved context even if an error has occured and also clear the error code. I'm asking that because I have experienced some bad behaviour of windows GDI API and because of that when I call 'cairo_restore()' the previously saved context, by 'cairo_save()', is not restored. It would be better if cairo library will allow developer to choose if it wants to restore or not the old context in the case of an error. By 'bad behaviour of windows GDI API' I mean that sometimes some GDI API returns error when should not (BitBlt(), CreateCompatibleDC() and others). The error codes are 6 and 8 (extracted with GetLastError()) which coresponds to the following error messages: "The handle is invalid" and "Not enough storage is available to process this command". I have searched on google about these errors and I discovered that others also experienced similar situations. Unfortunately I didn't find the real cause and a fix for this problem. Anyway, the ideea is that the application I'm working on it is not affected if these GDI functions doesn't do their job by failuring with the above error codes, but it is affected by the fact that 'cairo_restore()' doesn't restore the previously saved context because of these errors. Also it is necessary to clear the error code inside of 'cairo_restore()' because next time a cairo function will be called it will exit. For the application I'm working on it I have done the following modification for cairo library: void cairo_restore (cairo_t *cr) { ... // Reset the error code. We want to restore the old context even // if an error has occured cr->status = CAIRO_STATUS_SUCCESS; if (cr->gstate) { if (cr->gstate->target) { cr->gstate->target->status = CAIRO_STATUS_SUCCESS; } } //if (cr->status) //return; ... } This modification suits my application need. I was thinking maybe you can include such mechanism in the future versions of cairo library. Best regards, Iosif Haidu