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

Camera: Fix GI camera provider crash when no texture is available after loading #7049

Merged
merged 1 commit into from Dec 11, 2020

Conversation

Cheaterman
Copy link
Contributor

@Cheaterman Cheaterman commented Aug 18, 2020

Traceback:

   File "/home/cheaterman/Dev/Kivy/kivy/core/camera/camera_gi.py", line 137, in _update
     self.dispatch('on_load')
   File "kivy/_event.pyx", line 705, in kivy._event.EventDispatcher.dispatch
   File "kivy/_event.pyx", line 1248, in kivy._event.EventObservers.dispatch
   File "kivy/_event.pyx", line 1172, in kivy._event.EventObservers._dispatch
   File "/home/cheaterman/Dev/Kivy/kivy/uix/camera.py", line 112, in _camera_loaded
     self.texture_size = list(self.texture.size)
 AttributeError: 'NoneType' object has no attribute 'size'

Also see #6971.

@Cheaterman
Copy link
Contributor Author

Might seem a bit aggressive but since we can't assume the texture is assigned in on_load... Regarding texture size, I'm not sure how guaranteed we are it won't change dynamically (say when your phone flips from portrait to landscape?), so that might be a better idea either way ; but for self.texture, if we can assume the same texture will be reused across the CoreCamera lifetime (even through start()s and stop()s), we could make sure to only assign it once?

I'm not sure that's a safe assumption either, so here we are.

@Cheaterman
Copy link
Contributor Author

(FWIW I'm not quite sure how the Camera widget was invoked - I am using garden.zbarcam (which uses garden.xcamera) with default settings)

Copy link
Contributor

@wolfmanjm wolfmanjm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This indeed fixes the issue for me. My camera works again with master.

@Cheaterman
Copy link
Contributor Author

Cheaterman commented Aug 19, 2020 via email

def on_tex(self, *l):
def on_tex(self, camera):
self.texture = texture = camera.texture
self.texture_size = list(texture.size)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have to convert to list because texture_size is a ListProperty and assigning to it will convert texture.size to ObservableList.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, you don't have to set texture_size at all as it will be updated by on_texture method from Image class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can safely remove line self.texture_size = list(texture.size) from on_tex and _camera_loaded methods.

@@ -90,7 +90,9 @@ def __init__(self, **kwargs):
fbind('resolution', on_index)
on_index()

def on_tex(self, *l):
def on_tex(self, camera):
self.texture = texture = camera.texture
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can still check and update only when texture is None or when camera.texture.size is not equal to self.texture_size, right ?

@matham
Copy link
Member

matham commented Dec 10, 2020

Seems to have been fixed in #7071.

@matham matham closed this Dec 10, 2020
@Cheaterman
Copy link
Contributor Author

TFW ^^" anyway at least it's fixed. The line deletions I've made might still be a good idea though - mo' code mo' problems.

@wolfmanjm
Copy link
Contributor

FYI #7071 breaks gi based camera in 2.0.0, whereas this PR stops it crashing and allows it to work again.
I would recommend reverting #7071 and using this PR instead.

Copy link
Member

@matham matham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems correct as a texture may not exist at on_load.

@matham matham dismissed akshayaurora’s stale review December 11, 2020 23:02

It would be nice to only update if the texture object changed, but this is definitely working it shouldn't cause much performance loss.

@matham matham merged commit b816c2b into kivy:master Dec 11, 2020
@matham matham added the Component: Widgets kivy/uix, style.kv label Dec 11, 2020
@matham matham added this to the 2.1.0 milestone Dec 11, 2020
@matham matham changed the title Fix GI camera provider crash when no texture is available after loading Camera: Fix GI camera provider crash when no texture is available after loading Dec 11, 2020
@Cheaterman
Copy link
Contributor Author

Cheaterman commented Dec 12, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Widgets kivy/uix, style.kv
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants