evilmartians / oklch-picker

Color Picker for LCH
https://oklch.com
Other
849 stars 65 forks source link

Add 3d space #71

Closed muddv closed 1 year ago

muddv commented 1 year ago

This solves the same problem as #69 but has a few additional features (and maybe some additional flaws):

I want to work on displaying currently active color from the 2d graphs on 3d model, along with several small fixes, although this will still take some time, maybe it can be done before one of the prs is in condition to be merged. Maybe the best course of action is to work on this jointly, I hope this implementation has something to it.

ai commented 1 year ago

Can you show a screenshot or screen recording (sorry, I am on the go to run PR locally)

muddv commented 1 year ago

The page looks like this: image

muddv commented 1 year ago

Additionally: Medium screen layout: image Small screen layout: image

ai commented 1 year ago

Can you rebase to the main to fix pnpm audit error?

ai commented 1 year ago

There is a problem with a model. Did you create from set of slides?

Captura desde 2023-02-11 02-22-42

muddv commented 1 year ago

Can you rebase to the main to fix pnpm audit error?

Resolved it. This reveals a problem with package size limit. Correct me if I'm wrong, it measures all scripts, including 3d. But if 3d is disabled, in network tab the size is almost the same as in main branch. Maybe a different test condition can be made as opposed to simply increasing size limit.

muddv commented 1 year ago

There is a problem with a model. Did you create from set of slides?

What kind of problem do you mean exactly? The model is made up from points converted from rgb (p3 rgb etc.). If you mean that bottom is not solid, there are some fixed that can be tried.

ai commented 1 year ago

Resolved it. This reveals a problem with package size limit. Correct me if I'm wrong, it measures all scripts, including 3d. But if 3d is disabled, in network tab the size is almost the same as in main branch. Maybe a different test condition can be made as opposed to simply increasing size limit.

Yes. Can we set some prefix for the 3D bundle and move it out of Size Limit check by something like !plot-*.js?

ai commented 1 year ago

What kind of problem do you mean exactly?

The problem is that the space doesn’t look like a solid structure in the bottom.

ai commented 1 year ago

There is still a problem with solid structure in the bottom (screenshot from latest commit)

Captura desde 2023-02-11 17-56-57

There is also a problem with moving rotating space by mouse. Looks like the rotation center is not in the center of the space.

Captura desde 2023-02-11 17-57-59

Also, current space is very small. Can you make it bigger and put just above all card (we anyway will not keep it as small card)?

ai commented 1 year ago

A few other problems:

  1. There is no keyboard control of the space rotation.
  2. P3/Rec2020 toggles to break the current position of the space (space is going to origin position).
muddv commented 1 year ago

There is still a problem with solid structure in the bottom (screenshot from latest commit)

Yes it was kind of a quick hack, I want to adjust point generation math further to make it look better.

P3/Rec2020 toggles to break the current position of the space (space is going to origin position)

I think this can be paired with syncing the current position on the 2d charts with 3d space

Also, current space is very small. Can you make it bigger and put just above all card (we anyway will not keep it as small card)?

Can 3d space take up position of current LCH charts + settings on large screen? I think it can look good this way.

ai commented 1 year ago

Yes, we fixed the problem with solid surface of the model. What we still have:

  1. Strange ; file
  2. Bad mouse control. Seems like rotation center is out of the model.
  3. We can zoom too close (need to add limit).
  4. The bottom of surface looks wrong.

How it looks now: Captura desde 2023-02-14 20-27-24

How it should look (like Hue graph on C=0): Captura desde 2023-02-14 20-29-18

ai commented 1 year ago

Next changes to fix:

  1. P3/Rec2020 checkbox should not reset zoom and position.
  2. The rotation control is not very easy. Seems like rotation center is not inside the model and controls are not very intuitive (compare to globe on sitnik.ru)

Grabación de pantalla desde 2023-02-15 18-21-51.webm

muddv commented 1 year ago

I think most of the issues mentioned previously are fixed.

Is there anything else major that I'm missing?

muddv commented 1 year ago

Is there anything else major that I'm missing?

Sorry missed previous comment, some of the questions are already answered

ai commented 1 year ago

Nope, we are very close. I asked designers for design but I will add it by my own.

Let’s finish LCH, camera position on P3/Rec2020. It will be also nice to improve controls (see comment above).

muddv commented 1 year ago

Camera position is now remembered between renders. It would be easy to also add it to url if ?3d is included. Would it be a good idea to add it?

ai commented 1 year ago

While we are waiting for design, can we add a current slices to the space?

ai commented 1 year ago

@muddv here is a preview of design on top of your feature:

https://ai-oklch--pr85-v1-1-znr8xs52.web.app/#70,0.1,128,100

Do you have Twitter account to mention you in release tweet?

muddv commented 1 year ago

Looks great, thank you! My twitter is @null_preference, would appreciate a mention!