MaddTheSane / Simple-Comic

macOS comic viewer
MIT License
261 stars 23 forks source link

Find - Add a Find command to Simple Comic based on recognized text. #84

Closed DavidPhillipOster closed 2 years ago

DavidPhillipOster commented 2 years ago

I'm happy to answer any questions you may have about this pull request.

In short: Apple's NSTextFinder is a standard U.I. for a find command, but I could not use it because it expects that the app already has a complete text document. SimpleComic just has the text for the current page. So, I re-implemented the U.I. in OCRVision/OCRFindViewController, which talks to the actual Find engine OCRVision/OCRFind. The actual work of doing the Find is basically -[NSString rangeOfString:…] but that method doesn't handle starts-with-word and ends-with-word so I added them in a category in OCR_NSString. Since that category is doing the actual work, I added a unit test.

Since Apple's Vision framework uses a completion callback, I can't just write:

for(page in pages){
  selection = Find(OCRText(page)); 
  if (selection){ NavigateTo(page); Show(selection); break; } 
}

Instead, I have to turn the for loop inside out - that's what OCRRangeEnumerator is for:

func completion(matchingText){
  if(matchingText){
    NavigateTo(page); Show(matchingText);
  } else {
     page = enumerator.Next()
     if (page) {
      OCRText(page, completion);
     } else {
        NotFound();
     }
  }
}
OCRText(enumerator.Next(), completion);

But, I have to schedule the call to OCRText with dispatch_async to keep from overflowing the stack, so I need to put up a modal progress dialog to that the user can't change the Find operation while it is running. As long as I've got a progress dialog, it has a cancel button so the user can cancel a long running Find.

Since all of this depends on 'Recognize Text' being turned on in Preferences, turning 'Recognize Text' off modifies the Edit menu to remove the inappropriate menu items. (Turning 'Recognize Text' back on restores those items.)

nickv2002 commented 2 years ago

@DavidPhillipOster Thanks for all your work on this exciting feature! In another thread you talked about making a beta version of the app for people to try. Would it make sense to do that for this update?

DavidPhillipOster commented 2 years ago

Done. Thanks for the nudge. https://github.com/DavidPhillipOster/Simple-Comic/releases/tag/1.9.4β - It's my first time doing a release on github, so please get in touch with me if there are problems.

nickv2002 commented 2 years ago

I checked some files and did some searches with the beta version you posted. Works very well on my M1 MBA thanks!

Let's leave it up here for a week or so for other to test and then do a release to the App store version etc.

PS neat job on the icon. 😄

DavidPhillipOster commented 2 years ago

I'm glad you liked it.

nickv2002 commented 2 years ago

@DavidPhillipOster sorry I've been busy and I'm late to follow up here:

I tested the build you posted again recently and did some searches but couldn't get it to find any words that I typed in. I was trying many combinations including words that were clearly on the page and correctly able to be selected and copied. I was really confused and then I realize it was searching with case-sensitivity. Once I switched to Ignore Case it worked as expected again. I'm not sure if I had switched it off or if I had simply had the right case in my previous searches.

So since:

  1. Comic book speech bubbles etc are often written in all caps.
  2. MacOS text recognition often gets letter case wrong

It's probably better to just have it always be case-insensitive search. I think that would have minimal impact to the feature and avoid confusion as to why it didn't find what was expected.

If you can fix that, I'm happy to push out a new App Store build.

Thanks again for the awesome contribution!

DavidPhillipOster commented 2 years ago

Commit f30a0e8 should do the trick for you. Thanks for the review!

nickv2002 commented 2 years ago

Merged to MaddTheSane:find. Will merge back to arc branch after some App Store tweaks...