Skip to content

Enhancements for image scanning#29

Open
DreierF wants to merge 18 commits intoklep:masterfrom
DreierF:enhancements_for_image_scanning
Open

Enhancements for image scanning#29
DreierF wants to merge 18 commits intoklep:masterfrom
DreierF:enhancements_for_image_scanning

Conversation

@DreierF
Copy link

@DreierF DreierF commented Dec 30, 2019

Hi Scott,

Thanks for your work on this tool.
I found it very useful and a good starting point for my own needs. I'm currently digitalizing a bunch of old printed photos. To make this easier I added some enhancements to scanline that might also be useful for others.

Enhancements are:

  • An option to specify a specific area that should be scanned for flatbed scanners
  • An option to specify a creation date that should be stored in the EXIF metadata
  • An option to auto-trim whitespace surrounding the image
  • An option to automatically strip additional pixels from the trimmed image (which allows to cut off borders)

Additionally I have done some refactorings/cleanup:

  • Update to Swift 5
  • Each class has it's own file
  • Removed some duplicated code

Let me know what you think 🙂

P.S. I would suggest to look at the changes commit by commit.

@klep
Copy link
Owner

klep commented Dec 31, 2019

Thanks for the PR @DreierF! This actually includes a suggestion I just made on #30 to make the config object a little smarter! I'll have some time to take a closer look in the coming days.

Copy link
Owner

@klep klep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great enhancements! Some minor comments and questions, but looks great!

// scanline
//
// Created by Florian Dreier on 12/30/19.
// Copyright © 2019 Scott J. Kleper. All rights reserved.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should probably go and remove this everywhere, maybe just pointing to the LICENSE file. Feel free to omit this line. My name doesn't need to be on everything!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just the default header that XCode generated. I replaced the headers now with

//
//  This code is part of scanline and published under MIT license.
//

WDYT?


extension Array where Iterator.Element == Int {

func doesValueAroundIndexPassFivePercent(index: Int, total: Int) -> Bool {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment for this function would be helpful. I'm not 100% sure I get what it's doing

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't like this method either. Took this as a chance to refactor the code so that the method is no longer needed. Hope this makes things clearer.

@@ -0,0 +1,18 @@
//
// UIImageExtensions.swift
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong filename :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

import Foundation

/// Describes insets that can be applied to a rectangle.
struct Insets {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this just be UIEdgeInsets?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICS UIEdgeInsets is only part of UIKit, which is not linked against scanline as it is a command line application.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right...

@DreierF DreierF requested a review from klep January 4, 2020 20:11
@DreierF
Copy link
Author

DreierF commented Jan 4, 2020

Thanks for your review!

Now opens all scanned images in a single window
@"synonyms": @[@"size"],
@"type": @"string",
@"description": @"Specify the size of the area that should be scanned in cm (Default is the whole flatbed scan area)"
@"description": @"Specify the size of the area that should be scanned in cm. Forat is <width>x<height>, e.g. 20x20 (Default is the whole flatbed scan area)"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*Format :)

guard error == nil else {
logger.log("Error received while attempting to open a session with the scanner.")
delegate?.scannerControllerDidFail(self)
if(configuration.batchScan) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if(configuration.batchScan) {
if configuration.batchScan {


logger.log("Error received while attempting to close a session with the scanner.")
delegate?.scannerControllerDidFail(self)
if(configuration.batchScan) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if(configuration.batchScan) {
if configuration.batchScan {

#import <XCTest/XCTest.h>

#import "ScanConfiguration.h"
#import "../scanline/ScanConfiguration.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to get it to work by doing the following:

  1. Change to #import "ScanConfiguration.h" in ScanConfigurationTests.m
  2. Remove everything except ScanConfigurationTests.m and ScanConfiguration.m from the Compile Sources step
  3. Remove #import "scanline-Swift.h" from ScanConfiguration.m

It's not ideal -- we shouldn't need any of the app source files in the test target -- but I'm not sure what the issue is right now with that.

import Foundation

/// Describes insets that can be applied to a rectangle.
struct Insets {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, right...


init(fromString insets: String) {
let insetValues = insets.split(separator: ":")
self.init(left: Double(insetValues[0])!, top: Double(insetValues[1])!, right: Double(insetValues[2])!, bottom: Double(insetValues[1])!)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to avoid the force unwraps here?


CFRunLoopRun()

print("Done")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough...

@rramphal
Copy link

Hey @klep, I noticed that this PR was last updated in 2020, but there was a recent v2.0 release this year. I'm not sure how many enhancements were made between v1.0 and v2.0, but I was wondering where that leaves this PR and its enhancements. Thank you!

@klep
Copy link
Owner

klep commented Apr 26, 2022

@rramphal It looks like there were some comments that were not addressed. These do look like worthy changes, but they'd have to be adapted for the newer version...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants