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

[WIP] Up to Date ColorPicker UI Element #651

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

saran820
Copy link

@saran820 saran820 commented Aug 4, 2022

This PR is adding ColorPicker based on the older PR #20.

Missing components:

  • Resolving interactivity
  • Testing ColorPicker

Guidance is needed regarding interactivity. I have resolved the errors which stem from older versions. However, the ColorPicker is displayed on the window but has no responsiveness.

image

This was the code I used to get the above window:

from fury import ui, window
from fury.data import fetch_viz_icons
import sys

cp = ui.ColorPicker(position=(0, 0))

rect = ui.Rectangle2D(size=(100, 100), position=(400, 400), color=(1, 0, 1))

current_size = (800, 800)
show_manager = window.ShowManager(size=current_size,
                                  title="Colorpicker on Rectangle")

show_manager.scene.add(cp)
show_manager.scene.add(rect)

interactive = True

if interactive:
    show_manager.start()

window.record(show_manager.scene, size=current_size, out_path="viz_colorpicker.png")

@m-agour
Copy link
Contributor

m-agour commented Aug 4, 2022

Update: I see now this is still a draft, sorry for rushing. but please consider my notes while you implement this.

Hello @saran820,
Thanks for this PR. I know it's not functional yet, but I have some notes:
Below is a smooth widely used color picker that is well interpolated. While here in this PR the picker colors are not well interpolated. Not sure if this can be easily solved or not.
image ]image

Also, the side of the picker is very small, and no way to control its size while initialization.

@saran820
Copy link
Author

saran820 commented Aug 5, 2022

Hi @m-agour, thank you for your reply.
I will look into improving the interpolation as you suggested. But at the moment I am just trying to make the Colorpicker fully functional.
Additionally, there is a parameter called 'side' which when I was trying to resolve the code, I removed, and then I forgot to add before I put in the draft request. After trying it, the Colorpicker 'Side' parameter works but the lack of interactivity still persists.

image

@m-agour
Copy link
Contributor

m-agour commented Aug 5, 2022

Hi @m-agour, thank you for your reply. I will look into improving the interpolation as you suggested.

Thank you!

But at the moment I am just trying to make the Colorpicker fully functional.

Yeah, not rush on this.

Additionally, there is a parameter called 'side' which when I was trying to resolve the code, I removed, and then I forgot to add before I put in the draft request. After trying it, the Colorpicker 'Side' parameter works but the lack of interactivity still persists.
image

Looks good!

@skoudoro
Copy link
Contributor

Hi @saran820,

Thank you for doing this.

@ganimtron-10 and @xtanion will get back to you as soon as possible

@saran820
Copy link
Author

saran820 commented Aug 13, 2022

Hi @saran820,

Thank you for doing this.

@ganimtron-10 and @xtanion will get back to you as soon as possible

Hi, apologies for the whole thing, I was trying to reply but that happened. Thank you @skoudoro! I will update in case of any updates.

@saran820 saran820 closed this Aug 13, 2022
@saran820 saran820 deleted the colorpicker branch August 13, 2022 23:32
@saran820 saran820 restored the colorpicker branch August 13, 2022 23:32
@saran820 saran820 reopened this Aug 13, 2022
@saran820
Copy link
Author

Hi, so I have been able to manage the change in color of the Colorpicker Square through the Hue Bar. But this change in color does not change the color of the shape in the picture. I feel I might be missing some important aspects during testing. Kindly guide me with the same.

from fury import ui, window
from fury.data import fetch_viz_icons
import sys

cp = ui.ColorPicker(side=300, position=(0, 0))

rect = ui.Rectangle2D(size=(100, 100), position=(400, 400))


def change_hue(rect):
    color = cp.current_hue()


rect.on_change = change_hue

current_size = (800, 800)
show_manager = window.ShowManager(size=current_size,
                                  title="Colorpicker on Rectangle")

show_manager.scene.add(cp)
show_manager.scene.add(rect)

interactive = True

if interactive:
    show_manager.start()

window.record(show_manager.scene, size=current_size, out_path="viz_colorpicker.png")

image

@ganimtron-10
Copy link
Contributor

ganimtron-10 commented Aug 17, 2022

Hello @saran820 ,
Thanks for this PR.
I had a look on this PR before and saw the interactivity issues. I was unable to give some time to debug it.

Here in your recent code some points to be noted are:

  1. rect (Rectangle2D) doesn't have any on_change hook due to which the changes aren't reflected.
  2. Also you are just setting the local color variable and not using it any where.

Solutions:
Create a callback function for the on_change hook of the ColorPicker.(because whenever you change the ColorPicker you have to reflect the changes on the rect)
Then set the new color to the color property of the rect. (ie. rect.color = {new color from colorpicker}).

I am currently not having access to my pc, so I was unable to share the modified code.
Whenever I get back, I will update this comment with the code till then try to use the above solution.

@m-agour
Copy link
Contributor

m-agour commented Aug 17, 2022

Hi @saran820 ,
As Praneeth mentioned above. It's not supposed to be used like this. I see that you already have on_change hook but it's too complicated as you are feeding a lot of data to it. you might just make it take current color or self. Also you should to make color property that returns the current color of the ColorPicker in RGB.
And should be finally used as follows

color_picker = ui.ColorPicker( position=(0, 0))
rect = ui.Rectangle2D(size=(100, 100), position=(400, 400))

def change_rect_color(picker):
    global rect
    rect.color = picker.color

color_picker.on_change = change_rect_color

With that being said, there is a problem with the color (I suspect the interpolation issue we discussed) see below the selected color is not the same color on the rect
image

@saran820
Copy link
Author

Hi @ganimtron-10 and @m-agour, thank you for the insights. I will look into it too and update in case of any progress.
Additionally, @m-agour in the screenshot of your comment does the circular cursor move and change the color of the rectangle?

@skoudoro skoudoro changed the title Up to Date ColorPicker UI Element [WIP] Up to Date ColorPicker UI Element Sep 6, 2022
Comment on lines +3566 to +3567
self.ColorSelectionSquare._add_to_scene(scene) # this line was changed
self.pointer._add_to_scene(scene) # this line was changed
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello @saran820 ,
I am not sure if you are still working on it or not but I figured out why the interaction wasn't working for Rectangle2D.

If you change these lines of code as suggested, the below code would work.

Suggested change
self.ColorSelectionSquare._add_to_scene(scene) # this line was changed
self.pointer._add_to_scene(scene) # this line was changed
self.ColorSelectionSquare.add_to_scene(scene) # this line was changed
self.pointer.add_to_scene(scene) # this line was changed

Modified Code:

from fury import ui, window
from fury.data import fetch_viz_icons
import sys
import numpy as np

cp = ui.ColorPicker(side=300, position=(0, 0))

rect = ui.Rectangle2D(size=(100, 100), position=(400, 400))


def change_rect_color(color_picker, r, g, b):
    rect.color = np.asarray(color_picker.hsv2rgb(color_picker.current_hue, color_picker.current_saturation,
                                                 color_picker.current_value)) / 255


cp.on_change = change_rect_color

current_size = (800, 800)
show_manager = window.ShowManager(size=current_size,
                                  title="Colorpicker on Rectangle")

show_manager.scene.add(cp)
show_manager.scene.add(rect)

interactive = True

if interactive:
    show_manager.start()

window.record(show_manager.scene, size=current_size,
              out_path="viz_colorpicker.png")

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

4 participants