Page MenuHome

Draggable Panels Appearance Update
AbandonedPublic

Authored by Campbell Barton (campbellbarton) on Jul 7 2014, 9:47 PM.

Details

Summary

This updates the method:

static void ui_draw_panel_dragwidget(const rctf *rect)

to draw more obvious handles

Problems with the old Handles:

  1. Barely visible
  2. Function isn't clear (ambiguity with editor sizing)

Benefits of the new Handles:

  1. Obvious function
  2. Stands out/is visible (possibly too much?)

This is designed to scale with both DPI and Panel zooming, so it will look good across multiple screen sizes/pixel densities
Old:


------REV #1------

Zooming:

------REV #2------

------REV #3------

------REV #4------

------REV #5------

NOTE: This does not change anything but the way the handles and Panels are drawn

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
  • Changes from code review

I went ahead and did the changes, also did some fixes, because initial code used some extra compiler features that didn't work here. Uploaded a new version with slightly different colors. Different variables should be used instead of adding and then subtracting on the same variable because the color might be saturated on adding and subtracting will not restore the original.

However (and slightly reluctant to say since patch author did all this work):

I seem to remember we wanted to go to another direction entirely with UI (I remember @William Reynish (monkeyboi)'s mockups here). All those lines add too much visual clutter in the end. Is such a visual cue really needed?

Antonis Ryakiotakis (psy-fi) edited edge metadata.
  • more tweaks, should be identical to old result now.

I agree with @Antonis Ryakiotakis (psy-fi) - recent changes:

Screenshot:

add visual clutter and are not so good aesthetically. And I don't really get why these changes were made - the whole idea of dividing the panels better was to suggest draggability without the drag-area GUI element, yet it is still present in the last screenshot. I already stated that making apparent what can be dragged without using the drag-area GUI elements is a bigger task - these panel frames are not needed at the moment.

I agree with @Paweł Łyczkowski (plyczkowski). The most recent version is much too cluttered.

I believe both concepts (the drag handle vs. the header) are good options, but we need to choose one or the other. My personal preference would be go with the drag handles right now as it's the smallest change that doesn't require a total overhaul of the existing panels.

If the panels are to be redesigned I would much prefer that we give more attention to the entire properties area (and others that are affected) to ensure that styling stays nice and consistent. This is something that could give a good visual improvement, but let's not get ahead of ourselves.

We can always apply the drag handle changes now and then continue the overall panel design work. The benefit to this is it's a smaller, incremental change. A lot of smaller incremental changes are far easier to get approved and make progress on than a big design overhaul.

Sorry for the delay (I was at a grad party with some of my hs friends), but @Antonis Ryakiotakis (psy-fi), what is the difference between glcolor4ub and glcolor4ubv?
I appreciate the help guys, very nice of you.
I agree with @Jonathan Williamson (carter2422), we could always wait to redesign the panels until after the next update or further in the future, so I am cool with using the original idea.

It's more of a nitpick, the other version does the same thing, just uses a pointer instead of passing all components yourself. If one plans to reuse all the color components I find the vector version is tidier.

@Antonis Ryakiotakis (psy-fi), with your last update (and probably all of mine), region overlap doesn't work with the panel body because it's not transparent. How should this be fixed?
Also, IDK if you committed already, but will you be using the simple widget as per @Paweł Łyczkowski (plyczkowski) or final windowed version?

Haven't committed, imo we still need to resolve if we want all of the functionality in this patch. I'm tending towards just pushing the handle part of the patch for now along with Pawel and Jonathan.

@Antonis Ryakiotakis (psy-fi), @Paweł Łyczkowski (plyczkowski), @venomgfx, what do you guys think about committing the current functionality as an improvement and then working on the overall panel design as a separate task?

For me it's still a -1 to have handlers on every single panel.

There's really no need to constantly communicate that every panel can be dragged at all times. Really, how often you rearrange your panels? I'd save that spot for other things (e.g. contextual menus like I mentioned before).

On a separate note: does this patch add the ability to click-drag from anywhere in the panel's header? I think this is a must, plus changing the cursor to "grab" while dragging (regardless of having the handler or not).

In D637#79, @venomgfx wrote:

For me it's still a -1 to have handlers on every single panel.

There's really no need to constantly communicate that every panel can be dragged at all times. Really, how often you rearrange your panels? I'd save that spot for other things (e.g. contextual menus like I mentioned before).

I agree with you here, I think the issue is that without a more thorough design update of the panels the Header-Only dragging is not going to work very well.

We already show grab handles on every single panel, and so this patch is no different, except that it makes them much nicer and more distinguished from the Window-splitting handles.

@Antonis Ryakiotakis (psy-fi), @Paweł Łyczkowski (plyczkowski), @venomgfx, what do you guys think about committing the current functionality as an improvement and then working on the overall panel design as a separate task?

Yup, good idea.

@venomgfx what's the point of proposing a complete redesign (which adds a ton of cluttering imho, the one with 3-4 px spacing + shadow i mean) within a very specific patch? We currently have drag widget for every single panel which is visually equal to the split editor function, the task is about to update this with a more pertinent one.

This very same topic has been covered months ago and other devs said it's ok as a "quick" update (and a deeper redesign kept for the future maybe) though never merged T34397

  • existing code uses block->aspect new code uses UI_DPI_FAC - is there an important reason for that?
  • When UI_DPI_FAC is used a few times it could be assigned a var: const float dpi_fac = UI_DPI_FAC;
  • This is a bit verbose, headrect.ymax - (headrect.xmax - headrect.xmin) + 3 * UI_DPI_FAC.
Can replace: headrect.xmax - headrect.xmin can be BLI_rctf_size_x(&headrect). same for y.

The block->aspect is related to the window zoom from what I can deduce, and the UI_DPI_FAC relates to the dpi scaling for denser screens. I think having it scale with both is more typical of similar UI elements, so I can implement that easily.

About BLI_rctf_size_x(&headrect), I had implemented that before my rev was commandeered and the patch wiped it out :P

I will also work on code style

  • Idk if you were talking about codestyle for the new version or old, so I did both. Hopefully I got what you were talking about.
  • Using block->aspect apparently causes popping in size changes or unusual warping of sizes due to rounding floats to integer pixel values because of the small nature of the boxes.
  • I changed out longhand for BLI_rectf_sizex|y() in the new version

The reason why I don't think it's a good idea to just change the widget now and plan a redesign for later, is that it feels like applying a band-aid over a weak design.

Documentation/Tutorials written/recorded for -2.71 will have the current diagonal lines. 2.72 would have the new widget. And +2.73 would have the new panels (if we get the design in time). Is confusing.

Reminds me of the discussion about having the quick colored wireframe option instead of making a real system based on rules for coloring.

This is just replacing the widget, more important parts such as header-wide spot for click-drag or the cursor change while dragging I mentioned has not even been discussed.

We shouldn't go for the quickest way first to get around it and then redesign, it confuses users. If a topic is scheduled for redesign then we should work on that, put all our energy in having a nice clean design with improved usability, not apply a band-aid in the meantime (especially knowing it's such a high profile UI feature).

NB: i know this is not a forum so i don't want to keep discussing more than this last post from me, but i follow development quite closely and think i'd share my 2c because there is people spending hours on tasks like this which get stopped or left apart.

I agree a deeper redesign would be beneficial in many areas, but this is the real problem to me. Why it needed this patch to start think about Panels and nobody of the plenty "UI team members" never proposed a redesign and got it done within a reasonable amount of time? Thanks to external contributors like @Julian Eisel (Severin) and @Blake Stacks (blakenator) we got these additions and discussions going on, like navigation icons in 3d view, tabs, widgets appearance.. otherwise everything UI related is almost stopped, tabs have been introduced months ago and got muted until @Julian Eisel (Severin) posted a patch yesterday, topbar tabs on hold, 3d view icons on hold, wireframe colors on hold, new defaults keymap on hold..now a micro cosmetic update like drag widget get probably stopped because "confuses users" "tutorials get outdated" and "we need a deeper refactor". Am I the only who see how inefficent is all this?

Two out of the three UI member designers have disappeared (one never appeared actually), Ton extremely busy, Brecht away...i keep hearing the "needs thorough redesign" but see nothing compared to the big amount of cooks in the UI kitchen.

This is just my opinion, I hope things will stabilize because i'm sorry but i find UI design and development to be a rather messy as it's now, compared to other Blender modules, and this will push away these nice contributions by external people.

@Marco G (marcog) Please don't discuss such matters here, this is not a forum. Points rised by @venomgfx are valid, and the final decision belongs to the module owners. UI is a delicate matter, so rushing into changes can have bad results.

@Marco G (marcog), like @Paweł Łyczkowski (plyczkowski) said, this isn't the place for these discussions. If you have any concerns about the current UI progress you can always email me personally (as the current UI lead) via jonathan@cgcookie.com or ask in IRC.

@venomgfx those are all very valid points. My concern is that pushing off for a full redesign often means it won't actually get done. There's a lot of areas that need or have needed a "full redesign" for months or years now. Thorough, complete redesigns are very difficult to get finished (from all aspects; design, scope, development). Focusing on refinement rather than revamping generally leads to far more progress and polish in my experience.

That's not to say that we shouldn't tackle redesigns, but I don't think we should block small, incremental progress for the sake of waiting on a more complete redesign. Unless that redesign is already in progress and has good steam.

@venomgfx, that being said, I'm not opposed to pushing for a panel redesign/update so long as we're not stamping out the steam that @Blake Stacks (blakenator) and @Julian Eisel (Severin) are building.

@venomgfx I think using the widget for now would be the best option until the next release because such a drastic change as the panel redesign proposed needs to undergo more testing before being release-ready.
Also, @venomgfx, I have attempted active drag widgets, but as of the current mouse tracking system, there is no feasible way to do this IMO.
Here's a previous post, see if you can get it working:

Ok. I have been toying with it, but have not found a good way to do it. The way I have tried to is globalize the mousex/y positions and pass the activation rect to the draw call and test the intersection with

BLI_recti_isect_pt(recti,int,int)

which hasn't been working.
heres my code so far:

Campbell Barton (campbellbarton) edited edge metadata.

updated - less verbose and code-style (please commandeer back)

Checked the patch, as far as I can see zooming and offset is not working at all. (made sure that part wasnt changed by my recent update).

This isn't totally simple to solve, but its really needed before we can accept.

  • Zoom in- the squares look strange and dark.
  • Zoom out - the offset of the squares is wrong (too far down).

Also, this changes color of the entire panel, is that intended? (not really a fan of this, isn't the patch supposed to only be changing the header?)

To clear things up, this is the scope and look of the patch that was accepted:

@Paweł Łyczkowski (plyczkowski) in that case, this patch breaks the current theme option:

Themes -> Properties -> Show Background (after applying this patch, turning it off does nothing)

This is the current patch at 144 DPI, assuming large dark dots aren't intended?

assuming large dark dots aren't intended

Nope. At high DPI it should behave just like the current drag areas:

Also this patch makes region overlap a bit pointless? (the panels now have no alpha), is this intended too?

On higher density screens, those larger dark dots will appear to be about the same (maybe a little large), which is what DPI is made for, correct? If we were to leave them the same at higher densities (4k monitors) they would become invisible. Also, Region overlap can be fixed with changing the colors a bit, Show Background can be added to an if-statement, offset scaling is dividing space amount by block->aspect. I will commandeer at lunch if no one has made these changes by then. :)

If we were to leave them the same at higher densities (4k monitors) they would become invisible.

As far as I know (I do not have a high density monitor) the current drag areas are left the same at higher densities, and they do not become invisible.

From the replies - its still not clear.

https://developer.blender.org/D637#101
Is this the intended look or not?

Also this should be a small patch, easy to review and accept.

I can get the patch, test on high-dpi display, test with different DPI values and ensure code is OK and commit.

But the way it is now:

  • Background color changes, messes with region overlap, breaks theme setting.
  • DPI seems to fail, but I cant tell if this is whats meant or not.

I would prefer if a simple patch like changing how handles draw not scope-creep. Just get it working and if you want to change other aspects of panels, do as seperate design issue.
At this point its probably easier for me to write myself then try to understand whats going on with the patch which seems half working.


Is this the intended look or not?

For me - no. This is the intended look at high DPI:

And this is the intended look at normal DPI:

I can't make a diff right now because I don't have the source, but here's the .c file

That Contains the working handles, and just the handles so it will be a small patch for @Jonathan Williamson (carter2422)

I made a mistake earlier, so here is the corrected diff. @Jonathan Williamson (carter2422), if you have a high density monitor, it would be a big help.

This revision is now accepted and ready to land.Jul 24 2014, 1:22 AM

Idk why it says I accepted it, because I didn't push the button.

@Paweł Łyczkowski (plyczkowski) Thanks for the clarification. regarding high DPI version, did others agree on this? To me its near invisible.

@Blake Stacks (blakenator), patch needed edits to build, edited and will update diff.

  • Zoomed in boxes still look strange & dark
  • Zoomed out they vanish.

ugh. uploaded wrong diff,. corrected.

Guys, can we agree on high-dpi image and low-dpi image before going any further with this patch?

Otherwise its not really easy to review. I had the impression this was near finished because @Jonathan Williamson (carter2422) accepted the revision, but seems like thats probably not the case?

To me its near invisible.

The visibility is similar to the current ones, if not a bit higher:

@Paweł Łyczkowski (plyczkowski), Your comparing zooming in, with hi-dpi (retina) display (which draws all lines 2x width), Its not quite the same.

And even if existing area is almost invisible, this isnt great reason to keep it hard to see IMHO. Having UI element that scales and is clearly visible at different sizes is quite reasonable.

Jonathan Williamson (carter2422) requested changes to this revision.Jul 24 2014, 6:27 PM
Jonathan Williamson (carter2422) edited edge metadata.

@Paweł Łyczkowski (plyczkowski) can you upload the confirmed final version of both regular and hi-dpi (retina) versions of the design? There's too much confusion in this thread, let's ensure everyone is on the same page.

Retina is just twice the pixel density within the same screen space. We need to ensure that it looks the same (so far as visibility and size) on both regular and retina screens. I should have caught this earlier, so my apologies for that.

Let's get this patch done, little tasks like this should not drag on so long and take so much time. It's particularly unfair to the likes of @Campbell Barton (campbellbarton) and @Blake Stacks (blakenator).

This revision now requires changes to proceed.Jul 24 2014, 6:27 PM

@Jonathan Williamson (carter2422) I did it here:

This is the intended look at high DPI:

And this is the intended look at normal DPI:

But according to @Campbell Barton (campbellbarton) the high DPI is not visible enough. I do not own a high DPI screen, so you are a bit on your own, but scaling the dots x2 while retaining their positions relative to the header's height (so they move further apart from each other - like is shown on my high DPI mockup) should be fine. It should even be better than the current solution, where the line thickness does not change, as far as I can see, in higher DPI's (that's assuming that the Retina mode works just like changing the DPI).

But according to @Campbell Barton (campbellbarton) the high DPI is not visible enough. I do not own a high DPI screen, so you are a bit on your own, but scaling the dots x2 while retaining their positions relative to the header's height (so they move further apart from each other - like is shown on my high DPI mockup) should be fine. It should even be better than the current solution, where the line thickness does not change, as far as I can see, in higher DPI's (that's assuming that the Retina mode works just like changing the DPI).

For retina images maybe check out this site: http://www.kylejlarson.com/blog/2012/creating-retina-images-for-your-website/ Just basically just need to do a mockup (for future tasks) at twice the DPI but same pixel space.

Finalizing Patch

As it stands right now, based on the latest version, there is just two issues that I'm aware that need fixed before we can close this out. I believe these two issues are due to the same bug, but I'm not certain.

1. Retina looks bad

Handles looks bad on Hi-DPI (retina) screens. None of us have a retina screen, but we can simulate this by setting the system DPI to 144. The issue is that the top layer of bright pixels do not scale equally with the shadow layer, resulting in the top layer being thin. Here is what they look at 144 DPI:

Here is what they should look like (approximately, just a photoshop mockup):

2. Scaling panels does not scale handles correctly

As with the retina version, when scaling panels the handle does not draw consistently. The issue is the same, that the top bright layer of pixels only seems to scale horizontally. This can be very clearly seen when scaling the panels very large:

Let's get these two things fixed and then @Campbell Barton (campbellbarton) can give final code blessing and commit.

OOHH MA GAWD! That's what you meant? Wow I thought you were talking about the individual box, not the highlight. Your past arguments make loads more sense now! haha XD

Blake Stacks (blakenator) edited edge metadata.

Fixes the scaling issue, the way they actually wanted!

Great! Thanks @Blake Stacks (blakenator). Confirmed the scaling/retina issue to be fixed here.

@Campbell Barton (campbellbarton) can you give a final code review and then commit to master if it passes your inspection?

This revision is now accepted and ready to land.Jul 24 2014, 8:33 PM

Basic review of latest patch.

  • Zooming out still makes invisible (as in, they completely dissipater)
glRect's don't ensure at least one pixel offsets or sizes so its possible that rectangles are sub-pixel size, I think this is the cause of the problem. They should be clamped U.pixelsize minimum, so they are visible on hi-dpi
  • Picky - but there seems to be some alignment jittering pixel offset, maybe could overlook this, but think its the reason that small handles vanish. related to previous issue.
  • There is no need to use GL_BLEND, just interpolate all (3?) colors once and use those.

Note, since this isn't using line drawing, hi-dpi display looks no different to zooming in or turning DPI up. however minimum draw-size should be increased (as noted above).

Campbell Barton (campbellbarton) requested changes to this revision.Jul 25 2014, 7:34 AM
Campbell Barton (campbellbarton) edited edge metadata.
This revision now requires changes to proceed.Jul 25 2014, 7:34 AM
Blake Stacks (blakenator) edited edge metadata.

Fix requested by @Campbell Barton (campbellbarton). Would've been sooner, had I figured out why the diffs weren't applying

I will create a separate task for the panel redesign so this can be closed

Nevermind, I just remembered I'm not supposed to do that.

Blake Stacks (blakenator) edited edge metadata.

Sorry about yet another email, but I noticed a problem with my last diff. the dots should only use the header color if it is enabled, otherwise use the window back color.

Applied rB046d7590a6d4ed6a879a7080e572985a593e4eef

With edits:

  • Was using 3x different kinds of zoom level, (dpifac/rectangle/aspect)... unnecessary confusion, simply use rectangle size as as before.
  • The color blending function is for painting - use simple tint function UI_GetColorPtrShade3ubv
  • Avoid int/float rounding issues, use all int's for drawing boxes.
This revision is now accepted and ready to land.Jul 28 2014, 7:05 AM