Fix layout issues in Settings > UI (issue #4033) #4034

svobs · 2022-11-03T04:23:28Z

I have read CONTRIBUTING.md
This implements/fixes issue Layout errors in Settings > UI tab #4033.

Description:
I've been getting better/faster at Interface Builder layouts, and felt compelled to fix this one. Mostly due to the fact that I left visual layout debugging enabled and was starting to get haunted by the big purple miasma surrounding the page.

This update:

Fixes the conflicting constraints for the OSC toolbar preview in both places it's displayed (see Layout errors in Settings > UI tab #4033).
Also fixes the additional constraint errors and warnings which popped up in Interface Builder shown below.
Slightly reduced the width of the box surrounding the toolbar, and added a couple more pixels of padding, so that when the toolbar fills up it looks nice and full. (icons still align to the right side of the box when it's not full)

(never mind that the order of the icons is different between these two shots; I just did a bunch of testing and ended up with different settings between shots)

low-batt · 2022-11-04T03:22:27Z

Testing the PR, I'm still geting errors in the Xcode console and I'm seeing purple?:

svobs · 2022-11-04T08:58:17Z

Well that is...really strange. I ran it again and tried changing several system appearance settings, and tried different displays and scalings...and I wasn't able to see purple again.

I suppose it does seem possible that two Macs might vary by a single pixel, for whatever reason...I really thought Apple had gotten that aspect nailed down. Maybe not.

Looking at your screenshot though, it looks like the constraints it's complaining about are different than the original problem, and after going back and checking, I did find two vertical "equals" constraints, priority 1000, which spanned the same basic two locations but within different views, and that looked like a likely culprit. So I made some changes and was able to eliminate one of them and loosen the horizontal constraints a bit.

I also found a couple of places I had missed before, where programmatic constraints were being used which were a couple pixels off, although those would have only affected drag & drop (inside the Customize popup). Updated those too.

So, I pushed the updates, and hopefully it's stopped complaining now. If not...hopefully there's no if not :)

low-batt · 2022-11-04T12:53:45Z

Still getting complaints. Looks like the height of the area showing the icons is a problem. This is the errors emitted.

Xcode Console

2022-11-04 08:44:23.272305-0400 IINA[19828:1809195] [Layout] Unable to simultaneously satisfy constraints:
(
    "<NSLayoutConstraint:0x600001b07570 NSImageView:0x12359bbc0.height == 22   (active)>",
    "<NSLayoutConstraint:0x600001b10fa0 V:[NSStackView:0x1235b4aa0]-(0)-|   (active, names: '|':NSView:0x1235b4870 )>",
    "<NSLayoutConstraint:0x600001b11450 V:|-(0)-[NSStackView:0x1235b4aa0]   (active, names: '|':NSView:0x1235b4870 )>",
    "<NSLayoutConstraint:0x600001b009b0 'NSStackView.Edge.Min.Bottom' V:[NSImageView:0x12359bbc0]-(0)-|   (active, names: '|':NSStackView:0x1235b4aa0 )>",
    "<NSLayoutConstraint:0x600001b00960 'NSStackView.Edge.Min.Top' V:|-(0)-[NSImageView:0x12359bbc0]   (active, names: '|':NSStackView:0x1235b4aa0 )>",
    "<NSAutoresizingMaskLayoutConstraint:0x600001b02940 h=-&- v=-&- NSView:0x1235b4870.minY == - 1   (active, names: '|':NSBox:0x1235b45c0'Box' )>",
    "<NSAutoresizingMaskLayoutConstraint:0x600001b03200 h=-&- v=-&- V:|-(1)-[NSView:0x1235b4870]   (active, names: '|':NSBox:0x1235b45c0'Box' )>",
    "<NSLayoutConstraint:0x600001b115e0 NSBox:0x1235b45c0'Box'.height >= 24   (active)>"
)

Will attempt to recover by breaking constraint 
<NSLayoutConstraint:0x600001b07570 NSImageView:0x12359bbc0.height == 22   (active)>

Set the NSUserDefault NSConstraintBasedLayoutVisualizeMutuallyExclusiveConstraints to YES to have -[NSWindow visualizeConstraints:] automatically called when this happens.  And/or, set a symbolic breakpoint on LAYOUT_CONSTRAINTS_NOT_SATISFIABLE to catch this in the debugger.
2022-11-04 08:44:23.272783-0400 IINA[19828:1809195] [Layout] Unable to simultaneously satisfy constraints:
(
    "<NSLayoutConstraint:0x600001b07f70 NSImageView:0x1235c4920.height == 22   (active)>",
    "<NSLayoutConstraint:0x600001b10fa0 V:[NSStackView:0x1235b4aa0]-(0)-|   (active, names: '|':NSView:0x1235b4870 )>",
    "<NSLayoutConstraint:0x600001b11450 V:|-(0)-[NSStackView:0x1235b4aa0]   (active, names: '|':NSView:0x1235b4870 )>",
    "<NSLayoutConstraint:0x600001b02210 'NSStackView.Edge.Min.Bottom' V:[NSImageView:0x1235c4920]-(0)-|   (active, names: '|':NSStackView:0x1235b4aa0 )>",
    "<NSLayoutConstraint:0x600001b021c0 'NSStackView.Edge.Min.Top' V:|-(0)-[NSImageView:0x1235c4920]   (active, names: '|':NSStackView:0x1235b4aa0 )>",
    "<NSAutoresizingMaskLayoutConstraint:0x600001b02940 h=-&- v=-&- NSView:0x1235b4870.minY == - 1   (active, names: '|':NSBox:0x1235b45c0'Box' )>",
    "<NSAutoresizingMaskLayoutConstraint:0x600001b03200 h=-&- v=-&- V:|-(1)-[NSView:0x1235b4870]   (active, names: '|':NSBox:0x1235b45c0'Box' )>",
    "<NSLayoutConstraint:0x600001b115e0 NSBox:0x1235b45c0'Box'.height >= 24   (active)>"
)

Will attempt to recover by breaking constraint 
<NSLayoutConstraint:0x600001b07f70 NSImageView:0x1235c4920.height == 22   (active)>

Set the NSUserDefault NSConstraintBasedLayoutVisualizeMutuallyExclusiveConstraints to YES to have -[NSWindow visualizeConstraints:] automatically called when this happens.  And/or, set a symbolic breakpoint on LAYOUT_CONSTRAINTS_NOT_SATISFIABLE to catch this in the debugger.
2022-11-04 08:44:23.279656-0400 IINA[19828:1809195] [Layout] Unable to simultaneously satisfy constraints:
(
    "<NSLayoutConstraint:0x600001b00f50 NSImageView:0x1235c5d00.height == 22   (active)>",
    "<NSLayoutConstraint:0x600001b10fa0 V:[NSStackView:0x1235b4aa0]-(0)-|   (active, names: '|':NSView:0x1235b4870 )>",
    "<NSLayoutConstraint:0x600001b11450 V:|-(0)-[NSStackView:0x1235b4aa0]   (active, names: '|':NSView:0x1235b4870 )>",
    "<NSLayoutConstraint:0x600001b02580 'NSStackView.Edge.Min.Bottom' V:[NSImageView:0x1235c5d00]-(0)-|   (active, names: '|':NSStackView:0x1235b4aa0 )>",
    "<NSLayoutConstraint:0x600001b023a0 'NSStackView.Edge.Min.Top' V:|-(0)-[NSImageView:0x1235c5d00]   (active, names: '|':NSStackView:0x1235b4aa0 )>",
    "<NSAutoresizingMaskLayoutConstraint:0x600001b02940 h=-&- v=-&- NSView:0x1235b4870.minY == - 1   (active, names: '|':NSBox:0x1235b45c0'Box' )>",
    "<NSAutoresizingMaskLayoutConstraint:0x600001b03200 h=-&- v=-&- V:|-(1)-[NSView:0x1235b4870]   (active, names: '|':NSBox:0x1235b45c0'Box' )>",
    "<NSLayoutConstraint:0x600001b115e0 NSBox:0x1235b45c0'Box'.height >= 24   (active)>"
)

Will attempt to recover by breaking constraint 
<NSLayoutConstraint:0x600001b00f50 NSImageView:0x1235c5d00.height == 22   (active)>

Set the NSUserDefault NSConstraintBasedLayoutVisualizeMutuallyExclusiveConstraints to YES to have -[NSWindow visualizeConstraints:] automatically called when this happens.  And/or, set a symbolic breakpoint on LAYOUT_CONSTRAINTS_NOT_SATISFIABLE to catch this in the debugger.
2022-11-04 08:44:23.281090-0400 IINA[19828:1809195] [Layout] Unable to simultaneously satisfy constraints:
(
    "<NSLayoutConstraint:0x600001b02350 NSImageView:0x1235c6c20.height == 22   (active)>",
    "<NSLayoutConstraint:0x600001b10fa0 V:[NSStackView:0x1235b4aa0]-(0)-|   (active, names: '|':NSView:0x1235b4870 )>",
    "<NSLayoutConstraint:0x600001b11450 V:|-(0)-[NSStackView:0x1235b4aa0]   (active, names: '|':NSView:0x1235b4870 )>",
    "<NSLayoutConstraint:0x600001b02760 'NSStackView.Edge.Min.Bottom' V:[NSImageView:0x1235c6c20]-(0)-|   (active, names: '|':NSStackView:0x1235b4aa0 )>",
    "<NSLayoutConstraint:0x600001b00e10 'NSStackView.Edge.Min.Top' V:|-(0)-[NSImageView:0x1235c6c20]   (active, names: '|':NSStackView:0x1235b4aa0 )>",
    "<NSAutoresizingMaskLayoutConstraint:0x600001b02940 h=-&- v=-&- NSView:0x1235b4870.minY == - 1   (active, names: '|':NSBox:0x1235b45c0'Box' )>",
    "<NSAutoresizingMaskLayoutConstraint:0x600001b03200 h=-&- v=-&- V:|-(1)-[NSView:0x1235b4870]   (active, names: '|':NSBox:0x1235b45c0'Box' )>",
    "<NSLayoutConstraint:0x600001b115e0 NSBox:0x1235b45c0'Box'.height >= 24   (active)>"
)

Will attempt to recover by breaking constraint 
<NSLayoutConstraint:0x600001b02350 NSImageView:0x1235c6c20.height == 22   (active)>

Set the NSUserDefault NSConstraintBasedLayoutVisualizeMutuallyExclusiveConstraints to YES to have -[NSWindow visualizeConstraints:] automatically called when this happens.  And/or, set a symbolic breakpoint on LAYOUT_CONSTRAINTS_NOT_SATISFIABLE to catch this in the debugger.

svobs · 2022-11-05T02:46:04Z

Ack. Alright...interesting... It looks like on your Mac, either the interior height of the box isn't 22 pixels, or the exterior height isn't 24 pixels, as it is on mine. Let me see if I can come with something which doesn't require fixed heights for one of them...

svobs · 2022-11-05T06:16:17Z

OK. I redid the layout for the whole toolbar icon box, and parameterized the stuff inside it. It had better work now.

Here it is when I set the icon frame to 16x16:

And here it is with 50x50:

So it should be more than able to handle 24x24 now...

low-batt · 2022-11-05T17:40:35Z

iina/PrefOSCToolbarSettingsSheetController.swift

    dragItem.imageComponentsProvider = {
      let imageComponent = NSDraggingImageComponent(key: .icon)
      imageComponent.contents = self.buttonType.image()
-      imageComponent.frame = NSRect(origin: .zero, size: NSSize(width: 14, height: 14))
+      imageComponent.frame = NSRect(origin: .zero, size: NSSize(width: Preference.ToolBarButton.imageWidth, height: Preference.ToolBarButton.imageHeight))


I'm not understanding why the need to reference Preference.ToolBarButton.imageWidth and imageHeight. At this point imageComponent.contents contains the NSImage. Instead of creating a NSSize, can't this just be imageComponent.contents.size, referencing the NSImage size property?

low-batt · 2022-11-05T17:43:15Z

iina/PrefOSCToolbarDraggingItemViewController.swift

    dragItem.imageComponentsProvider = {
      let imageComponent = NSDraggingImageComponent(key: .icon)
      imageComponent.contents = self.buttonType.image()
-      imageComponent.frame = NSRect(origin: .zero, size: NSSize(width: 14, height: 14))
+      imageComponent.frame = NSRect(origin: .zero, size: NSSize(width: Preference.ToolBarButton.imageWidth, height: Preference.ToolBarButton.imageHeight))


I'm not understanding why the need to reference Preference.ToolBarButton.imageWidth and imageHeight. At this point imageComponent.contents contains the NSImage. Instead of creating a NSSize, can't this just be imageComponent.contents.size, referencing the NSImage size property?

low-batt · 2022-11-05T17:47:19Z

Worked fine this time, no purple. Just confused as to why the size can not be obtained directly from the image.

Glad you are tackling constraint issues. There are two known crashes due to constraint problems.

low-batt

Pulled locally and tested. Looks good to me.

svobs · 2022-11-06T06:38:30Z

Worked fine this time, no purple. Just confused as to why the size can not be obtained directly from the image.

Apparently it can be (at least now). Unlike my fix in #4029 where I scrutinized the layout of every component in the layout (have you had a chance to look at that?), with this one I haven't strayed too far from the parts which were causing obvious trouble.

You were right to point this out though. Due to the 14x14 constraint, a couple of the icons were getting squished while dragging. The pip icon is 16x14, and I'm actually not sure where Settings icon is coming from but it's larger than that.

low-batt · 2022-11-06T19:22:09Z

Totally overloaded. Been meaning to get to PR #4029. Just tested. Looks good to me.

Since we are talking about layout constraints in preferences panels, issue #3505 is a reproducible crash in the subtitles preferences due to constraints.

When you really dig deeply into software you find stuff. I'm surprised the icons are not all the same size. That seems like something that needs to be fixed.

…yout for toolbar preview so it will always fit the box properly

svobs · 2022-11-07T06:04:47Z

Totally overloaded. Been meaning to get to PR #4029. Just tested. Looks good to me.

No problem, and great! Was just a little nervous because I sunk an enormous amount of time and effort into that one and hadn't heard anything.

When you really dig deeply into software you find stuff. I'm surprised the icons are not all the same size. That seems like something that needs to be fixed.

Well.. Only today noticed: the Quick Settings icon in the player window's toolbar isn't scaled to the same size as the other icons.

I dug in deeper, and it looks like this icon is not consistently sized even in the player window: it's fine for my high DPI display, but slightly too small on my low-DPI display:

It looks like the fix for this won't be simple, because it's using a system icon, and as noted here, that's not a good idea. If we could agree to replace it with a nice custom-made gear icon, that would solve all our problems. But will leave that for another issue.

I pushed a final update for this one which managed to remove the 2 pixels of spacing I had added between each icon, so now the spacing is again identical to the MainWindow toolbar's.

Since we are talking about layout constraints in preferences panels, issue #3505 is a reproducible crash in the subtitles preferences due to constraints.

I'll take a look at this.

uiryuu

Visually good to me

uiryuu · 2022-11-15T02:22:48Z

Thanks!

svobs force-pushed the pr-pref-ui-view-layout-fixes branch from 2448295 to cd3a9aa Compare November 4, 2022 08:25

svobs force-pushed the pr-pref-ui-view-layout-fixes branch from cd3a9aa to 3f868f7 Compare November 5, 2022 06:12

low-batt reviewed Nov 5, 2022

View reviewed changes

svobs force-pushed the pr-pref-ui-view-layout-fixes branch from 3f868f7 to e3f0232 Compare November 6, 2022 06:14

low-batt approved these changes Nov 6, 2022

View reviewed changes

low-batt linked an issue Nov 6, 2022 that may be closed by this pull request

Layout errors in Settings > UI tab #4033

Closed

low-batt requested a review from uiryuu November 6, 2022 19:07

Fix layout errors for toolbar in OSC Prefs UI and popup. Also redo la…

1cd6afe

…yout for toolbar preview so it will always fit the box properly

svobs force-pushed the pr-pref-ui-view-layout-fixes branch from e3f0232 to 1cd6afe Compare November 7, 2022 06:17

uiryuu approved these changes Nov 8, 2022

View reviewed changes

uiryuu merged commit 2f6c5f6 into iina:develop Nov 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix layout issues in Settings > UI (issue #4033) #4034

Fix layout issues in Settings > UI (issue #4033) #4034

svobs commented Nov 3, 2022

low-batt commented Nov 4, 2022

svobs commented Nov 4, 2022

low-batt commented Nov 4, 2022

svobs commented Nov 5, 2022

svobs commented Nov 5, 2022 •

edited

low-batt Nov 5, 2022

low-batt Nov 5, 2022

low-batt commented Nov 5, 2022

low-batt left a comment

svobs commented Nov 6, 2022

low-batt commented Nov 6, 2022

svobs commented Nov 7, 2022

uiryuu left a comment

uiryuu commented Nov 15, 2022

Fix layout issues in Settings > UI (issue #4033) #4034

Fix layout issues in Settings > UI (issue #4033) #4034

Conversation

svobs commented Nov 3, 2022

low-batt commented Nov 4, 2022

svobs commented Nov 4, 2022

low-batt commented Nov 4, 2022

svobs commented Nov 5, 2022

svobs commented Nov 5, 2022 • edited

low-batt Nov 5, 2022

Choose a reason for hiding this comment

low-batt Nov 5, 2022

Choose a reason for hiding this comment

low-batt commented Nov 5, 2022

low-batt left a comment

Choose a reason for hiding this comment

svobs commented Nov 6, 2022

low-batt commented Nov 6, 2022

svobs commented Nov 7, 2022

uiryuu left a comment

Choose a reason for hiding this comment

uiryuu commented Nov 15, 2022

svobs commented Nov 5, 2022 •

edited