Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assertion errors in “newPixbufFromFile” if an error occurred. #75

Open
lscrd opened this issue May 28, 2020 · 7 comments
Open

Assertion errors in “newPixbufFromFile” if an error occurred. #75

lscrd opened this issue May 28, 2020 · 7 comments

Comments

@lscrd
Copy link

lscrd commented May 28, 2020

If the file we try to load in a Pixbuf doesn’t contain an image, an error is returned by Gtk but it is ignored in newPixbufFromFile. Thus, we are in an inconsistent state and the next instructions in newPixbufFromFile cause assertion errors (which can be seen if we launch the program from a terminal).

To avoid this, it would be necessary to exit prematurely from newPixbufFromFile if an error occurred. For instance, the proc could return nil instead of the expected Pixbuf. This is not perfect as we have no way to retrieve the error code in order to understand why the loading failed.

A better solution could be to raise an exception whose value contains the error code i.e. a glib.Error (GError).

For now, I have changed my code to check the type of the file I try to load, before actually loading it. But if an error occurs for some other reason, I’m still in trouble.

Note that the same problem exists for newPixbufAnimationFromFile.

@StefanSalewski
Copy link
Owner

Thanks for reporting.

I don't know much about Pixbuf, so I will have to consult the API docs.

Gintro uses for some functions Nim exeptions when GErros occur, so it should be possible to fix your Issue. I will try to investigate it soon.

@StefanSalewski
Copy link
Owner

Oh yes, it's a funny and simple bug:

For constructors like gdk_pixbuf_new_from_file()

we have 3 high level procs:

proc newPixbufFromFile*(filename: cstring): Pixbuf =

proc newPixbufFromFile*(tdesc: typedesc; filename: cstring): tdesc =

proc initPixbufFromFile*[T](result: var T; filename: cstring) {.deprecated.} =

Currently we do generate only for the last one the exception code.

Note, the last one, the init() variant is deprecated, it will be removed soon.

@lscrd
Copy link
Author

lscrd commented May 29, 2020

My bad. I didn’t look at the init() variant proc as I no longer use the deprecated versions. The variants with a typedesc parameter are much more elegant and convenient.

Indeed an exception is raised which is fine. But as it is raised only at the end of the procedure, if an error occurred we will try to continue the processing which will still cause assertion errors.

I think that the test for an error should be done before the call to g_object_get_qdata. I don’t see any drawback by doing this, but I may be wrong.

@StefanSalewski
Copy link
Owner

I think that the test for an error should be done before the call to g_object_get_qdata.

Indeed, I have discovered this also. Raising the exception should always occur as early as possible. Unfortunately I have to modify the code-generator a bit for that, which is not difficult but there is always the risk that it introduces new errors, for which we have not tests yet. So I have to be a bit cautious. I hope I can do these changes in the next few days.

@lscrd
Copy link
Author

lscrd commented May 29, 2020

Thanks. There is no urgency. I have other things to solve which are not related to gintro.

@StefanSalewski
Copy link
Owner

OK, the new generator script is available, please test with

nimble install gintro@#head

Unfortunately the shape of the generated modules has changed a lot, so it was hard to check them with diff command. My examples compiles and work, but I can not guarantee that there are no new errors.

@lscrd
Copy link
Author

lscrd commented Jun 1, 2020

This modification seems indeed to have changed a lot of code.

I modified slightly my current program in order to get to an exception. And, as expected, I got one if a file cannot be loaded. Then I added a try… except… to catch the exception and it worked. Of course, all the assert errors have disappeared which was the main reason for these modifications.

The program run as expected and all that previously worked still works. There was no bad surprise. So, for now, all is fine for me.

Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants