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

Added the watcher class to UI #448

Closed
wants to merge 6 commits into from

Conversation

antrikshmisri
Copy link
Member

@antrikshmisri antrikshmisri commented Jun 28, 2021

This PR adds support for a watcher class in the UI elements. The purpose of this class is to monitor a particular attribute from the UI element after it has been added to the scene. If the attribute changes in the real time, a user defined callback is triggered and the scene is force rendered. Is this something that the UI module needs? Below is the code example:-

from fury.ui import Panel2D
from fury import window

panel = Panel2D(size=(200, 200), position=(300, 300), color=(0.5, 0.3, 0.8))

current_size = (1000, 1000)
show_manager = window.ShowManager(size=current_size,
                                  title="Watcher Example")


def on_change(i_ren, obj, ui):
    print(f'{ui.watcher.attr} has changed from {ui.watcher.original_attr} to {ui.watcher.updated_attr}')


def change_opacity(i_ren, obj, panel_2d):
    panel_2d.opacity = 0.5

panel.watcher.callback = on_change
panel.background.on_left_mouse_button_clicked = change_opacity

show_manager.scene.add(panel)
show_manager.initialize()
panel.watcher.start(100, show_manager, 'opacity')

# To interact with the UI, set interactive = True
interactive = False

if interactive:
    show_manager.start()

@codecov
Copy link

codecov bot commented Jun 28, 2021

Codecov Report

Merging #448 (27f2810) into master (f2bb0b5) will decrease coverage by 0.27%.
The diff coverage is 45.03%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #448      +/-   ##
==========================================
- Coverage   88.31%   88.04%   -0.28%     
==========================================
  Files          31       31              
  Lines        6446     6483      +37     
  Branches      770      776       +6     
==========================================
+ Hits         5693     5708      +15     
- Misses        534      556      +22     
  Partials      219      219              
Impacted Files Coverage Δ
fury/actor.py 89.04% <2.32%> (ø)
fury/ui/helpers.py 61.90% <37.14%> (-30.96%) ⬇️
fury/pick.py 72.27% <72.46%> (ø)
fury/ui/core.py 92.08% <100.00%> (+0.03%) ⬆️
fury/window.py 80.16% <100.00%> (ø)

@skoudoro
Copy link
Contributor

Thank you for this PR @antrikshmisri.

I believe it can be useful and it would be good if many developers could try it before going further.

@Nibba2018 @guaje, could you test this new feature?

@antrikshmisri
Copy link
Member Author

I believe it can be useful and it would be good if many developers could try it before going further.

Sure, makes sense.

@Nibba2018
Copy link
Member

Hello, in my opinion I think this should be separate global watcher which could be user customizable instead of having separate watchers for each UI element. Also I think you can already perform such updates via callbacks itself.

@antrikshmisri Could you please provide an usage example so that its easier for us to understand. Thank you

@guaje
Copy link
Contributor

guaje commented Jun 29, 2021

Hi @antrikshmisri!

This seems to be a new cool feature, however, when I run the example you are providing it gives me the following error:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-1-3bd41f4f3e0b> in <module>
     16     panel_2d.opacity = 0.5
     17 
---> 18 panel.watcher.callback = on_change
     19 panel.background.on_left_mouse_button_clicked = change_opacity
     20 

AttributeError: 'Panel2D' object has no attribute 'watcher'

Could you please add a unit test?

@antrikshmisri
Copy link
Member Author

This seems to be a new cool feature, however, when I run the example you are providing it gives me the following error:

Hey @guaje, I have updated the PR, now it should work.

@antrikshmisri
Copy link
Member Author

Hello, in my opinion I think this should be separate global watcher which could be user customizable instead of having separate watchers for each UI element. Also I think you can already perform such updates via callbacks itself.

Hey @Nibba2018, I didn't quite get what you mean by a global watcher. Does it mean a class that would be initialized on the top and we can observe properties from different classes through that same instance. Something like:-

watcher = Watcher(some args)

element = UI_element(some_args)
element_1 = UI_element(some_args)

watcher.start(element, args)
watcher.start(element_1, args)

Also, yes you can do such things by callbacks but those need to binded with an event first or should be user hook provided by a event callback.

@antrikshmisri Could you please provide an usage example so that its easier for us to understand. Thank you

A good use case for this would be something like resizing all the elements inside a panel when the panel size changes. As panel size can be changed from the bottom right corner as well as from WindowResizeEvent, currently a user hook needs to be binded with both of these, this will become really annoying when more panel resizing events/callbacks are added. A better approach would be to just listen for changes in the size and execute a callback accordingly.

@guaje
Copy link
Contributor

guaje commented Jul 2, 2021

Hey @guaje, I have updated the PR, now it should work.

Thanks.

I see you have added many class methods in the proposed class. Could you add some tests to make sure everything in the class works as expected?

Also, could you create a test case with the code provided in the example?

As a thumbs-up, adding a parent class for all the UI components is something we've been missing and will help us to discriminate between actors, visual elements associated with them, and UI elements.

As @Nibba2018 suggested could you create a more detailed example or tutorial showing and explaining how this new feature works and why it is important. This could be a good starting point:

A good use case for this would be something like resizing all the elements inside a panel when the panel size changes. As panel size can be changed from the bottom right corner as well as from WindowResizeEvent, currently a user hook needs to be binded with both of these, this will become really annoying when more panel resizing events/callbacks are added. A better approach would be to just listen for changes in the size and execute a callback accordingly.

Finally, could you fix the conflict with fury/ui.py?

@antrikshmisri
Copy link
Member Author

I see you have added many class methods in the proposed class. Could you add some tests to make sure everything in the class works as expected?
Also, could you create a test case with the code provided in the example?

Sure, the tests will be up soon.

As @Nibba2018 suggested could you create a more detailed example or tutorial showing and explaining how this new feature works and why it is important. This could be a good starting point:

Sure, will create an example that demonstrates the use of the watcher class.

@skoudoro
Copy link
Contributor

skoudoro commented Jun 6, 2023

Closing, For now, this feature is not indeed and need more thinking.

Thank you for this @antrikshmisri

@skoudoro skoudoro closed this Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants