Page MenuHome
Differential D3979

Make input cursor theme-able
Needs ReviewPublic
Actions

Authored by Roel Koster (kostex) on Nov 22 2018, 8:59 PM.

Details

Reviewers
Brecht Van Lommel (brecht)
Summary

Hi,

I would like to suggest to make the edit input cursor theme-able. Attached is an image of what I mean.
If it weren't for my age, I wouldn't care, but with all the (dark) theme-ing going on, seeing contrasts is getting worse for me...
The cursor I'm talking about wasn't theme-able. Now it is, together with a setting to make it wider.

Now a guy at my age can see it again ;-)

Ps. Ton, this is why I asked where to find the cursor on IRC if you remember.. Was quite a search but:
dan heb ik ook weer wat te doen :-b

N.B.

  • Patch is for blender 2.80 branch of course.
  • I don't know if the padding of the structs is ok, couldn't quite grasp that.

Diff Detail

Repository
rB Blender

Event Timeline

Aaron Carlisle (Blendify) added a subscriber: Aaron Carlisle (Blendify).Nov 24 2018, 5:59 AM

I dont think we need another theme option for this, I think this could be done better automated way.

Brecht Van Lommel (brecht) requested changes to this revision.EditedNov 26 2018, 12:54 AM

I'm not sure this needs to be theme-able either to be honest. We should not go overboard with the number of theming options.

It seems we are not taking into account DPI for cursor width, and I guess you have a high DPI macOS screen? So if we handle that properly it will already be double the size by default. Maybe the size could also increase from 2 to 3, if it doesn't overlap the text characters too much.

This revision now requires changes to proceed.Nov 26 2018, 12:54 AM
Roel Koster (kostex) added a comment.Nov 26 2018, 7:55 AM

I don't know your specific ages, but please bear in mind that not everything you see, is what others see.. I can't distinguish that cursor from the background because of it's color.. and secondarily it's width.
And yes, I'm on a 5k screen.
As I can change the background color, sooner or later the contrast is gone.. you can't theme one thing and leave out it's opposite.
It was a suggestion others would enjoy too.. on rightclick select it gained a couple of likes..

If the "theme everything" approach is something you want to avoid, I really don't see why that list is enormous as it is right now.. and main developers are throwing switches and what not in there like there is no tomorrow (albeit pre-beta)
Anyhow, It was a suggestion. Nothing is keeping me from using it myself ;-)

Cheers,
Roel

Brecht Van Lommel (brecht) added a comment.Nov 26 2018, 3:43 PM

At the very least there should be an attempt to solve this problem by default. If it's an obscure option in the theme settings then only a small percentage of users who need this will actually be helped by it, because they will never think to look for it.

We can make the cursor width correctly take into account DPI, maybe make it one pixel wider, and change the default color to red like the text editor. Maybe that works well enough for everyone?

Roel Koster (kostex) added a comment.Nov 26 2018, 4:08 PM

As long as you agree that there needs to be a contrasty and hidpi friendly cursor, then I'm all for it ;-)
A bit "jammer" that there is no support for customization via theming, but hey.. in the end that doesn't matter.

Thanks for taking the time to hear me out.
Roel

Roel Koster (kostex) requested review of this revision.Dec 10 2018, 12:34 PM

Hi.. Since you're opening up themable colors again (see https://developer.blender.org/D3949) I'd like to re-propose my patch..

In other words: You see now what I mean with how hard it is for "outsiders" to get a perfectly feasible option integrated, and how easy it is when you're a friend of the family?
I don't want to push my patch in any way... it's your code, you make the choices here.. I understand that.. but please be more open in the future and just say: we don't want your crap, instead of discarding it with a flimsy excuse that you've got enough settings already.
I could live with clear language. But this freaks me out a bunch!

Brecht Van Lommel (brecht) added a comment.EditedDec 10 2018, 4:45 PM

There is no rule against theme colors, but there has to be a good reason for them.

Please see my comment above, where I give suggestions for how an automatic solution to this problem might work, without the need for theme colors and also helpful for those not aware of obscure theme settings. That is what I think should be tried.

Brecht Van Lommel (brecht) requested changes to this revision.Dec 10 2018, 4:47 PM
This revision now requires changes to proceed.Dec 10 2018, 4:47 PM
Brecht Van Lommel (brecht) added a comment.Dec 10 2018, 5:31 PM

If that doesn't work well enough, there could still be a theme color by the way. But there is clearly a DPI bug here causing part of the problem. And when adding an option the question is always, can we do better by default?

So far you have not engaged with my feedback about that, which is what this patch is waiting for.

Roel Koster (kostex) updated this revision to Diff 12851.EditedDec 11 2018, 8:56 AM

I wasn't aware that I was/am the one to apply those proposals.
Anyway, I've search the source and came up with a possible fix, to discard the "themable width".
Since the console and text editor both have a themable cursor color, I kept it in as well for the edit cursor.
I've applied the cursor width calculation to those two as well b.t.w.

Hopefully the UI_DPI_FAC is the one that will work.. at the moment I have no access to an HiDPI screen

Roel Koster (kostex) removed a reviewer: Ton Roosendaal (ton).Sun, Jun 2, 12:18 PM
Roel Koster (kostex) updated this revision to Diff 15724.

Quite some time has passed.
I'm over the fact that you will not make the cursor themeable, but please, at the least accept the minimal change of keeping in line with UI_DPI_FAC.. on high dpi screens the cursor is not visible..
And the cursor color: red.. well.. I can't force you, but please reconsider a more contrasty color than the current one..

But in the end, with the possibility of changing the background/select color of every widget and not being able to select the color of the superimposed cursor, makes no sense to me.

Thanks for taking the time to hear me out,
Roel

Revision Contents
Changeset List

Diff 15724

View Options

source/blender/editors/interface/interface_widgets.c

Loading...