Added crop functionality by TheTripleV · Pull Request #926 · WPIRoboticsProjects/GRIP

TheTripleV · 2019-03-22T04:13:52Z

A CropOperation has been added that allows an image to be cropped down to a smaller size.
Users can specify a location and size for the crop. They can also specify the location of the rectangle from which the crop propogates: top left, bottom left, top right, bottom right, center.

Note: I don't know how I would go about writing/testing the code generation.

Example usage:

JLLeitschuh · 2019-03-22T18:29:37Z

core/src/main/java/edu/wpi/grip/core/operations/composite/CropOperation.java

+            height.intValue()
+            );
+
+    final Mat output = new Mat(input, regionOfInterest).clone();


This can lead to a memory leak. @SamCarlberg Do you know of a good class to use as an example of how this should be done to avoid the memory leak?

I don't think the clone is necessary. The constructor used here uses the same memory as the base image IIRC so there's only minimal overhead. I'll have to take a look when I'm at my computer

Well, we don't want this operation modifying the original input matrix.

JLLeitschuh · 2019-03-22T18:30:34Z

This is awesome! We'll try to get to it. Can you work on fixing the build so that CI passes?
Looks like you have Checkstyle violations.

TheTripleV · 2019-03-23T02:32:46Z

I fixed the 2 [WhitespaceAfter] errors I believe. However, the [CustomImportOrder] errors are the same ones I see in ResizeOperation (I used ResizeOperation as a starter file).

Also, based on another fork I just saw, I refactored the perform method to not create a new mat every iteration.

TheTripleV · 2019-03-23T02:33:23Z

Right now, I'm just running the Google Checkstyle. Is there a custom checkstyle file for GRIP?

codecov-io · 2019-03-23T02:49:07Z

Codecov Report

❗ No coverage uploaded for pull request base (master@2910a91). Click here to learn what that means.
The diff coverage is 63.82%.

@@            Coverage Diff            @@
##             master     #926   +/-   ##
=========================================
  Coverage          ?   52.93%           
  Complexity        ?        1           
=========================================
  Files             ?      330           
  Lines             ?     8939           
  Branches          ?      562           
=========================================
  Hits              ?     4732           
  Misses            ?     4002           
  Partials          ?      205

core/src/main/java/edu/wpi/grip/core/operations/composite/CropOperation.java

JLLeitschuh · 2019-03-25T18:49:43Z

Right now, I'm just running the Google Checkstyle. Is there a custom checkstyle file for GRIP?

I haven't actually developed in this codebase in so long that I honestly forget. @SamCarlberg can you help out here?

SamCarlberg · 2019-03-25T18:52:02Z

Yes there is: https://github.com/WPIRoboticsProjects/GRIP/blob/master/checkstyle.xml

Run gradlew checkstyleMain to run checkstyle on the production sourcesets, or gradlew check to run all code checks (checkstyle, PMD, ...) as well as unit tests. This is what we have CI use to check pull requests.

SamCarlberg · 2019-04-18T02:11:29Z

@TheTripleV With the recent addition of CUDA support, you'll need to change the Mat sockets to use MatWrapper

I can add the code generation in a followup if you're not comfortable with it

TheTripleV · 2019-04-24T19:11:07Z

Cool. I'll take a look at it after my finals next week. I'm just unsure on running tests on GRIP in general. For the operation files, I see test cases/files written. But for code generation, do we just run each piece of generated code manually?

TheTripleV · 2019-05-14T06:05:35Z

I updated the crop operation to use MatWrapper. Interestingly, the operation performs just fine without a clone. This is because every operation that occurs after a crop makes a copy of the data in the MatWrapper.

Should I leave the .clone() out for performace (less memory copies) or should it be in for safety's sake?

JLLeitschuh · 2019-05-14T14:56:50Z

@SamCarlberg ^^

SamCarlberg · 2019-05-15T21:05:01Z

You can safely omit calling clone()

JLLeitschuh · 2019-05-16T18:33:56Z

core/src/main/java/edu/wpi/grip/core/operations/composite/CropOperation.java

+    if (input.isCpu()) {
+      output.set(input.getCpu().apply(regionOfInterest)); // .clone() optional
+    } else {
+      output.set(input.getGpu().apply(regionOfInterest)); // .clone() optional


I don't think the .clone() comments are necessary.

-Fixed incorrect Socket Name -Checkstyle errors?

-If memory is in GPU, keep it there

TheTripleV · 2019-05-19T04:08:03Z

I added Java Code Generation. One weird thing I haven't been able to figure out is how it sets default values.

	public void process(Mat source0) {
		// Step Crop0:
		Mat cropInput = source0;
		double cropX = 100;
		double cropY = 100;
		double cropWidth = 50;
		double cropHeight = 50;
		Origin cropOrigin = Center;
		crop(cropInput, cropX, cropY, cropWidth, cropHeight, cropOrigin, cropOutput);

	}

In this step, Origin cropOrigin = Center; should be Origin cropOrigin = Origin.get("Center"); similar to Blur's code generation.

SamCarlberg · 2019-05-19T17:28:12Z

There's a hardcoded macro in ui/src/main/resources/edu/wpi/grip/ui/codegeneration/java/macros.vm for doing the enum lookups:

#elseif ($input.type().equals("BlurType"))
		$input.type() ${tMeth.name($input.name())} = ${input.type()}.get("${input.value()}");

You should be able to add an 'or' there to generate the expected enum lookup

#elseif ($input.type().equals("BlurType") || $input.type().equals("Origin"))

- Allow Top Left Origin to be accessible

JLLeitschuh · 2020-07-14T14:52:32Z

Sorry for letting this sit for so long. Happy to merge if it's in a working state.

JLLeitschuh

LGTM!

JLLeitschuh · 2020-08-17T18:27:53Z

Thanks for your contribution @TheTripleV!!

JLLeitschuh reviewed Mar 22, 2019

View reviewed changes

JLLeitschuh reviewed Mar 25, 2019

View reviewed changes

core/src/main/java/edu/wpi/grip/core/operations/composite/CropOperation.java Outdated Show resolved Hide resolved

TheTripleV force-pushed the crop_operation branch from fcd0bec to 76ee236 Compare May 14, 2019 06:02

JLLeitschuh reviewed May 16, 2019

View reviewed changes

TheTripleV added 8 commits May 18, 2019 13:58

Added crop functionality

d30942f

-Refactored operation

a3d3579

-Fixed incorrect Socket Name -Checkstyle errors?

Reordered imports

05b7dd5

Clarify usage of apply() method on Mat

29d7792

Clarify usage of apply() method on Mat

3d3f105

Refactor use of Mats to MatWrapper

c478302

-If memory is in GPU, keep it there

checkstyle compliance

ad5ea29

Removed unneeded comments.

cc708b3

TheTripleV force-pushed the crop_operation branch from 07e1e1d to cc708b3 Compare May 18, 2019 17:58

Java Code Generation

83eebda

TheTripleV added 3 commits May 19, 2019 21:59

Added macro to fix Java Code Generation

16bb85d

Complete incomplete comment

19dc585

Python code generation implemented

680ea82

TheTripleV added 3 commits May 23, 2019 23:43

Python Code Gen Fix

3a97d17

- Allow Top Left Origin to be accessible

C++ Code Generation Implemented

9b082d7

Remove unneeded memory copy

b83475e

JLLeitschuh approved these changes Aug 17, 2020

View reviewed changes

JLLeitschuh merged commit 40cdbc1 into WPIRoboticsProjects:master Aug 17, 2020

Comments

Conversation

TheTripleV commented Mar 22, 2019

Uh oh!

JLLeitschuh Mar 22, 2019

Choose a reason for hiding this comment

Uh oh!

SamCarlberg Mar 22, 2019

Choose a reason for hiding this comment

Uh oh!

JLLeitschuh Mar 22, 2019

Choose a reason for hiding this comment

Uh oh!

JLLeitschuh commented Mar 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheTripleV commented Mar 23, 2019

Uh oh!

TheTripleV commented Mar 23, 2019

Uh oh!

codecov-io commented Mar 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

JLLeitschuh commented Mar 25, 2019

Uh oh!

SamCarlberg commented Mar 25, 2019

Uh oh!

SamCarlberg commented Apr 18, 2019

Uh oh!

TheTripleV commented Apr 24, 2019

Uh oh!

TheTripleV commented May 14, 2019

Uh oh!

JLLeitschuh commented May 14, 2019

Uh oh!

SamCarlberg commented May 15, 2019

Uh oh!

JLLeitschuh May 16, 2019

Choose a reason for hiding this comment

Uh oh!

TheTripleV commented May 19, 2019

Uh oh!

SamCarlberg commented May 19, 2019

Uh oh!

JLLeitschuh commented Jul 14, 2020

Uh oh!

JLLeitschuh left a comment

Choose a reason for hiding this comment

Uh oh!

JLLeitschuh commented Aug 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JLLeitschuh commented Mar 22, 2019 •

edited

Loading

codecov-io commented Mar 23, 2019 •

edited

Loading