From df0bd3793b0b00dfe7714bb4986dbf5aba3e86db Mon Sep 17 00:00:00 2001 From: undecoded-coder Date: Thu, 13 Jan 2022 10:03:37 -0700 Subject: [PATCH 1/6] View-based outline view mostly working. Outstanding issues: - Selection is lost during reloads (and causes issues deselecting!) --- .../Interface/English.lproj/LDrawDocument.xib | 34 +++++++- .../Application/Document/LDrawDocument.h | 2 +- .../Application/Document/LDrawDocument.m | 85 +++++++++++-------- .../Source/LDraw/Support/LDrawGLRenderer.m | 2 +- 4 files changed, 84 insertions(+), 39 deletions(-) diff --git a/trunk/Bricksmith/Resources/Interface/English.lproj/LDrawDocument.xib b/trunk/Bricksmith/Resources/Interface/English.lproj/LDrawDocument.xib index 6da62ac..cab93c9 100644 --- a/trunk/Bricksmith/Resources/Interface/English.lproj/LDrawDocument.xib +++ b/trunk/Bricksmith/Resources/Interface/English.lproj/LDrawDocument.xib @@ -58,7 +58,7 @@ - + @@ -70,12 +70,41 @@ - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -939,6 +968,7 @@ + diff --git a/trunk/Bricksmith/Source/Application/Document/LDrawDocument.h b/trunk/Bricksmith/Source/Application/Document/LDrawDocument.h index ad985c1..c640cb4 100644 --- a/trunk/Bricksmith/Source/Application/Document/LDrawDocument.h +++ b/trunk/Bricksmith/Source/Application/Document/LDrawDocument.h @@ -44,7 +44,7 @@ // class LDrawDocument // //////////////////////////////////////////////////////////////////////////////// -@interface LDrawDocument : NSDocument +@interface LDrawDocument : NSDocument { IBOutlet DocumentToolbarController *toolbarController; IBOutlet NSObjectController *bindingsController; diff --git a/trunk/Bricksmith/Source/Application/Document/LDrawDocument.m b/trunk/Bricksmith/Source/Application/Document/LDrawDocument.m index 32d9937..7e72196 100644 --- a/trunk/Bricksmith/Source/Application/Document/LDrawDocument.m +++ b/trunk/Bricksmith/Source/Application/Document/LDrawDocument.m @@ -3636,8 +3636,8 @@ - (id)outlineView:(NSOutlineView *)outlineView //**** NSOutlineViewDataSource **** //========== outlineView:objectValueForTableColumn:byItem: ===================== // -// Purpose: Returns the representation of item given for the given table -// column. +// Purpose: Returns the representation of the given item for the given table +// column, as either a string or attributed string object. // //============================================================================== - (id) outlineView:(NSOutlineView *)outlineView @@ -3919,48 +3919,37 @@ - (BOOL)outlineView:(NSOutlineView *)outlineView #pragma mark - #pragma mark Delegate -//**** NSOutlineView **** -//========== outlineView:willDisplayCell:forTableColumn:item: ================== -// -// Purpose: Returns the representation of item given for the given table -// column. -// -//============================================================================== -- (void) outlineView:(NSOutlineView *)outlineView - willDisplayCell:(id)cell - forTableColumn:(NSTableColumn *)tableColumn - item:(id)item -{ +//========== outlineView:viewForTableColumn:item: ============================== +/// Returns the representation of the given item for the given table column. +- (NSView *)outlineView:(NSOutlineView *)outlineView viewForTableColumn:(NSTableColumn *)tableColumn item:(id)item { + NSTableCellView *cellView = [outlineView makeViewWithIdentifier:@"FileOutlineCellView" owner:nil]; + NSString *imageName = nil; NSImage *theImage; if([item isKindOfClass:[LDrawDirective class]]) imageName = [item iconName]; - + if(imageName == nil || [imageName isEqualToString:@""]) theImage = nil; else theImage = [NSImage imageNamed:imageName]; - - [(IconTextCell *)cell setImage:theImage]; -}//end outlineView:willDisplayCell:forTableColumn:item: - + cellView.imageView.image = theImage; + + return cellView; +} //**** NSOutlineView **** //========== outlineViewSelectionDidChange: ==================================== -// -// Purpose: We have selected a different something in the file contents. -// We need to show it as selected in the OpenGL viewing area. -// This means we may have to change the active model or step in -// order to display the selection. -// -//============================================================================== +/// We have selected a different something in the file contents. We need to show +/// it as selected in the OpenGL viewing area. This means we may have to change +/// the active model or step in order to display the selection. - (void)outlineViewSelectionDidChange:(NSNotification *)notification { - NSOutlineView *outlineView = [notification object]; + NSOutlineView *outlineView = notification.object; NSArray *selectedObjects = [self selectedObjects]; - id lastSelectedItem = [outlineView itemAtRow:[outlineView selectedRow]]; + id lastSelectedItem = [outlineView itemAtRow:outlineView.selectedRow]; LDrawMPDModel *selectedModel = [self selectedModel]; LDrawStep *selectedStep = [self selectedStep]; NSInteger selectedStepIndex = 0; @@ -3971,19 +3960,19 @@ - (void)outlineViewSelectionDidChange:(NSNotification *)notification // selecting parts can trigger OpenGL commands, we should make sure we have // a context active, but we should also restore the current context when // we're done. - NSOpenGLContext *originalContext = [NSOpenGLContext currentContext]; + NSOpenGLContext *originalContext = NSOpenGLContext.currentContext; [[LDrawApplication sharedOpenGLContext] makeCurrentContext]; //Deselect all the previously-selected directives // (clears the internal directive flag used for drawing) - for(counter = 0; counter < [self->selectedDirectives count]; counter++) - [[selectedDirectives objectAtIndex:counter] setSelected:NO]; + for(counter = 0; counter < self->selectedDirectives.count; counter++) + [selectedDirectives[counter] setSelected:NO]; //Tell the newly-selected directives that they just got selected. [selectedDirectives release]; selectedDirectives = [selectedObjects retain]; - for(counter = 0; counter < [self->selectedDirectives count]; counter++) - [[selectedDirectives objectAtIndex:counter] setSelected:YES]; + for(counter = 0; counter < self->selectedDirectives.count; counter++) + [selectedDirectives[counter] setSelected:YES]; // Update things which need to take into account the entire selection. // The order matters: the search panel unregisters itself as the active colorwell @@ -4011,7 +4000,22 @@ - (void)outlineViewSelectionDidChange:(NSNotification *)notification } } } - [[self documentContents] noteNeedsDisplay]; + + // Issue: in *view-based* outlines and tables, calling -reloadData (as in + // -[self docChanged] which receives this notification) will destroy and + // recreate the cell views on every reload, which gets rid of selection + // highlighting. Since we know we don't need to reload every time we make a + // selection anyways, adding a userInfo flag for this (and checking it) + // works for the most part, but it leaves a bug where now and then selecting + // a part in the user interface and dragging it (even if its position isn't + // changed) will still cause us to reload and our selection disappears. Not + // only that, but we are unable to deselect the part by clicking on the + // viewport background for some reason (somehow tied to the outline view?). + // So this workaround isn't *quite* enough yet. + [NSNotificationCenter.defaultCenter + postNotificationName:LDrawDirectiveDidChangeNotification + object:[self documentContents] + userInfo: @{ @"isSelectionChange" : @YES }]; //See if we just selected a new part; if so, we must remember it. if ([lastSelectedItem isKindOfClass:[LDrawPart class]] || @@ -4675,6 +4679,8 @@ - (void)drawerWillOpen:(NSNotification *)notification // notification on our document after many change notifications // on parts. See docChanged for more. // +// Notification: LDrawDirectiveDidChangeNotification +// //============================================================================== - (void) partChanged:(NSNotification *)notification { @@ -4695,7 +4701,8 @@ - (void) partChanged:(NSNotification *)notification // to refresh drawing, and we ned it to redo our menus. NSNotification * doc_notification = [NSNotification notificationWithName:LDrawDirectiveDidChangeNotification - object:docContents]; + object:docContents + userInfo:@{ @"isSelectionChange" : @YES }]; // Notification is queued and coalesced; [[NSNotificationQueue defaultQueue] @@ -4719,12 +4726,20 @@ - (void) partChanged:(NSNotification *)notification // operation. This is where we do the expensive stuff like // resync the hierarchy and update the menus. // +// Notification: LDrawDirectiveDidChangeNotification +// //============================================================================== - (void)docChanged:(NSNotification *)notification { // This functionality was in partChanged through Bricksmith 3.0. + + // NOTE: This method is currently being called **twice** per call (at least + // for outline selections) — once before -partChanged: and once afterward. + [fileContentsOutline selectObjects:selectedDirectives]; + if (!notification.userInfo[@"isSelectionChange"]) [fileContentsOutline reloadData]; + [self updateInspector]; diff --git a/trunk/Bricksmith/Source/LDraw/Support/LDrawGLRenderer.m b/trunk/Bricksmith/Source/LDraw/Support/LDrawGLRenderer.m index 828e90a..d139b0a 100644 --- a/trunk/Bricksmith/Source/LDraw/Support/LDrawGLRenderer.m +++ b/trunk/Bricksmith/Source/LDraw/Support/LDrawGLRenderer.m @@ -1811,7 +1811,7 @@ - (void) activeModelDidChange:(NSNotification *)notification [self->delegate LDrawGLRendererNeedsRedisplay:self]; -}//end displayNeedsUpdating +}//end activeModelDidChange From 62dd1434ee20a34ddfda6e5485dbdcbd623d729c Mon Sep 17 00:00:00 2001 From: undecoded-coder Date: Thu, 13 Jan 2022 10:28:17 -0700 Subject: [PATCH 2/6] Filter "Unparsable META command" output to keep it from flooding the console every time a part is selected. --- trunk/Bricksmith/Source/Application/General/RelatedParts.m | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/trunk/Bricksmith/Source/Application/General/RelatedParts.m b/trunk/Bricksmith/Source/Application/General/RelatedParts.m index 5e00ddc..d2c293a 100644 --- a/trunk/Bricksmith/Source/Application/General/RelatedParts.m +++ b/trunk/Bricksmith/Source/Application/General/RelatedParts.m @@ -332,6 +332,13 @@ - (id) initWithFilePath:(NSString *)filePath else { #if DEBUG + // Where related.ldr includes comments throughout, it ends up + // filling the console with output we probably don't care about. + // Filtering for the "!" prefix may be overly generic if we want to + // actually catch something unexpected here, but as the majority of + // meta commands we could possibly care about include the prefix, it + // seems like a good balance. + if ([parsedField containsString:@"!"]) printf("Unparsable META command: %s\n", [orig_line UTF8String]); #endif } From 26c71d3cc50ecd308659a08b3b6635684c088c99 Mon Sep 17 00:00:00 2001 From: undecoded-coder Date: Sun, 13 Feb 2022 11:19:56 -0700 Subject: [PATCH 3/6] Fix selection in the file outline view! (and some other changes) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Also set outlive view cell text from delegate method instead of using Cocoa Bindings (for consistency with icon image using the same method) • Remove code from previous attempts to fix outline view selection • Minor stylistic changes --- .../Interface/English.lproj/LDrawDocument.xib | 11 +-- .../Application/Document/LDrawDocument.h | 5 ++ .../Application/Document/LDrawDocument.m | 83 +++++++++---------- .../Source/LDraw/Support/LDrawDirective.m | 4 +- .../Source/Widgets/LDrawFileOutlineView.m | 4 +- 5 files changed, 52 insertions(+), 55 deletions(-) diff --git a/trunk/Bricksmith/Resources/Interface/English.lproj/LDrawDocument.xib b/trunk/Bricksmith/Resources/Interface/English.lproj/LDrawDocument.xib index cab93c9..2170731 100644 --- a/trunk/Bricksmith/Resources/Interface/English.lproj/LDrawDocument.xib +++ b/trunk/Bricksmith/Resources/Interface/English.lproj/LDrawDocument.xib @@ -51,14 +51,14 @@ - + - + @@ -84,7 +84,7 @@ - + @@ -94,9 +94,6 @@ - - - @@ -968,7 +965,7 @@ - + diff --git a/trunk/Bricksmith/Source/Application/Document/LDrawDocument.h b/trunk/Bricksmith/Source/Application/Document/LDrawDocument.h index c640cb4..8f52cde 100644 --- a/trunk/Bricksmith/Source/Application/Document/LDrawDocument.h +++ b/trunk/Bricksmith/Source/Application/Document/LDrawDocument.h @@ -86,6 +86,11 @@ NSArray * markedSelection; // if we are mid-marquee selection, this is an array of the previously selected directives before drag started } +/// Whether the document is in the process of updating the current selection in +/// the outline view. This is used to prevent an infinite loop of selection +/// change callbacks, specifically in -[LDrawDocument docChanged:]. +@property (nonatomic) BOOL isMidSelection; + // Accessors - (LDrawFile *) documentContents; - (NSWindow *)foremostWindow; diff --git a/trunk/Bricksmith/Source/Application/Document/LDrawDocument.m b/trunk/Bricksmith/Source/Application/Document/LDrawDocument.m index 7e72196..fe4c17f 100644 --- a/trunk/Bricksmith/Source/Application/Document/LDrawDocument.m +++ b/trunk/Bricksmith/Source/Application/Document/LDrawDocument.m @@ -3923,6 +3923,8 @@ - (BOOL)outlineView:(NSOutlineView *)outlineView /// Returns the representation of the given item for the given table column. - (NSView *)outlineView:(NSOutlineView *)outlineView viewForTableColumn:(NSTableColumn *)tableColumn item:(id)item { NSTableCellView *cellView = [outlineView makeViewWithIdentifier:@"FileOutlineCellView" owner:nil]; + // Either an NSString, or an NSAttributedString. + id theText = [self outlineView:outlineView objectValueForTableColumn:tableColumn byItem:item]; NSString *imageName = nil; NSImage *theImage; @@ -3936,6 +3938,7 @@ - (NSView *)outlineView:(NSOutlineView *)outlineView viewForTableColumn:(NSTable theImage = [NSImage imageNamed:imageName]; cellView.imageView.image = theImage; + cellView.textField.objectValue = theText; return cellView; } @@ -4000,22 +4003,7 @@ - (void)outlineViewSelectionDidChange:(NSNotification *)notification } } } - - // Issue: in *view-based* outlines and tables, calling -reloadData (as in - // -[self docChanged] which receives this notification) will destroy and - // recreate the cell views on every reload, which gets rid of selection - // highlighting. Since we know we don't need to reload every time we make a - // selection anyways, adding a userInfo flag for this (and checking it) - // works for the most part, but it leaves a bug where now and then selecting - // a part in the user interface and dragging it (even if its position isn't - // changed) will still cause us to reload and our selection disappears. Not - // only that, but we are unable to deselect the part by clicking on the - // viewport background for some reason (somehow tied to the outline view?). - // So this workaround isn't *quite* enough yet. - [NSNotificationCenter.defaultCenter - postNotificationName:LDrawDirectiveDidChangeNotification - object:[self documentContents] - userInfo: @{ @"isSelectionChange" : @YES }]; + [[self documentContents] noteNeedsDisplay]; //See if we just selected a new part; if so, we must remember it. if ([lastSelectedItem isKindOfClass:[LDrawPart class]] || @@ -4088,8 +4076,8 @@ - (void) LDrawGLViewMouseNotPositioning:(LDrawGLView *)glView //**** LDrawGLView **** //========== LDrawGLView:acceptDrop: =========================================== // -// Purpose: The user has deposited some drag-anddrop parts into an -// LDrawGLView. Now they need to be imported into the model. +// Purpose: The user has deposited some drag-and-drop parts into an +// LDrawGLView. Now they need to be imported into the model. // // Notes: Just like in -duplicate: and // -outlineView:acceptDrop:item:childIndex:, we appropriate the @@ -4684,7 +4672,7 @@ - (void)drawerWillOpen:(NSNotification *)notification //============================================================================== - (void) partChanged:(NSNotification *)notification { - LDrawDirective *changedDirective = [notification object]; + LDrawDirective *changedDirective = notification.object; LDrawFile *docContents = [self documentContents]; // Since the document sends out part changes too, make sure it isn't the doc @@ -4701,11 +4689,10 @@ - (void) partChanged:(NSNotification *)notification // to refresh drawing, and we ned it to redo our menus. NSNotification * doc_notification = [NSNotification notificationWithName:LDrawDirectiveDidChangeNotification - object:docContents - userInfo:@{ @"isSelectionChange" : @YES }]; + object:docContents]; // Notification is queued and coalesced; - [[NSNotificationQueue defaultQueue] + [NSNotificationQueue.defaultQueue enqueueNotification:doc_notification postingStyle:NSPostASAP coalesceMask:NSNotificationCoalescingOnName|NSNotificationCoalescingOnSender @@ -4732,28 +4719,36 @@ - (void) partChanged:(NSNotification *)notification - (void)docChanged:(NSNotification *)notification { // This functionality was in partChanged through Bricksmith 3.0. - - // NOTE: This method is currently being called **twice** per call (at least - // for outline selections) — once before -partChanged: and once afterward. - - [fileContentsOutline selectObjects:selectedDirectives]; - if (!notification.userInfo[@"isSelectionChange"]) - [fileContentsOutline reloadData]; - - - [self updateInspector]; - - // Technically we don't need to redo the model menu on every UI edit. In the - // future we should add specific notifications or directive observations to - // detect this case. But 3.0 and earlier ran this once for every edit (when - // part changes were escalated to doc changes) and then twice on real model - // changes (because we'd hit the case on the model and again on the doc). - // - // In practice it doesn't matter - for now, for the test cases we have, - // rebuilding the menus is relatively cheap. A user can only add so many - // MPD parts to a file without going completely insane. - [self addModelsToMenus]; - + // ------------------------------------------------------------- + // Due to using a view-based outline (post-Bricksmith 3.1), this had to + // change in order to work properly with the behavioral change of outline + // views regarding selection across calls to -reloadData. + + // We don't want -selectObjects: below to bring us to an infinite loop + // because the outline view is sending out *another* selection changed + // notification! + if (!self.isMidSelection) { + self.isMidSelection = YES; + [fileContentsOutline reloadData]; + + [self updateInspector]; + + [fileContentsOutline selectObjects:selectedDirectives]; + + // Technically we don't need to redo the model menu on every UI edit. In the + // future we should add specific notifications or directive observations to + // detect this case. But 3.0 and earlier ran this once for every edit (when + // part changes were escalated to doc changes) and then twice on real model + // changes (because we'd hit the case on the model and again on the doc). + // + // In practice it doesn't matter - for now, for the test cases we have, + // rebuilding the menus is relatively cheap. A user can only add so many + // MPD parts to a file without going completely insane. + [self addModelsToMenus]; + + // Let's not forget to allow future changes! + self.isMidSelection = NO; + } }//end docChanged: diff --git a/trunk/Bricksmith/Source/LDraw/Support/LDrawDirective.m b/trunk/Bricksmith/Source/LDraw/Support/LDrawDirective.m index b64c962..ea337a1 100644 --- a/trunk/Bricksmith/Source/LDraw/Support/LDrawDirective.m +++ b/trunk/Bricksmith/Source/LDraw/Support/LDrawDirective.m @@ -785,10 +785,10 @@ - (BOOL)isAncestorInList:(NSArray *)containers //============================================================================== - (void) noteNeedsDisplay { - [[NSNotificationCenter defaultCenter] + [NSNotificationCenter.defaultCenter postNotificationName:LDrawDirectiveDidChangeNotification object:self]; -}//end setNeedsDisplay +}//end noteNeedsDisplay //========== registerUndoActions: ============================================== diff --git a/trunk/Bricksmith/Source/Widgets/LDrawFileOutlineView.m b/trunk/Bricksmith/Source/Widgets/LDrawFileOutlineView.m index c3c3bf4..a8d6b6b 100644 --- a/trunk/Bricksmith/Source/Widgets/LDrawFileOutlineView.m +++ b/trunk/Bricksmith/Source/Widgets/LDrawFileOutlineView.m @@ -43,9 +43,9 @@ - (NSIndexSet *) selectObjects:(NSArray *)objects NSInteger counter = 0; //Gather up the indices of the pasted objects. - for(counter = 0; counter < [objects count]; counter++) + for(counter = 0; counter < objects.count; counter++) { - currentObject = [objects objectAtIndex:counter]; + currentObject = objects[counter]; indexOfObject = [self rowForItem:currentObject]; [indexesToSelect addIndex:indexOfObject]; } From 328eee0de0519651767b9f059b090b8eca75fc0f Mon Sep 17 00:00:00 2001 From: undecoded-coder Date: Sun, 13 Feb 2022 12:21:05 -0700 Subject: [PATCH 4/6] Use source list (aka sidebar) styling for file contents outline view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Also updates outline view cells to use auto layout constraints to ensure proper sizing when using Source List styling --- .../Interface/English.lproj/LDrawDocument.xib | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/trunk/Bricksmith/Resources/Interface/English.lproj/LDrawDocument.xib b/trunk/Bricksmith/Resources/Interface/English.lproj/LDrawDocument.xib index 2170731..7edab42 100644 --- a/trunk/Bricksmith/Resources/Interface/English.lproj/LDrawDocument.xib +++ b/trunk/Bricksmith/Resources/Interface/English.lproj/LDrawDocument.xib @@ -37,7 +37,7 @@ - + @@ -51,21 +51,20 @@ - + - + - + - - + - + @@ -77,25 +76,29 @@ - - - + + - - - + + - - - + + - + + + + + + + + @@ -110,9 +113,10 @@ +