jankovicsandras / imagetracerjava

Simple raster image tracer and vectorizer written in Java for desktop
The Unlicense
134 stars 49 forks source link

layering, if statements that are always true #3

Closed MiYanni closed 8 years ago

MiYanni commented 8 years ago

In the function, the loops (i & j) start at 1. However, within the loops, you are checking to see if the values are > 0. The values will never not be > 0. I was simply wondering what the intent was behind these checks. Thanks! :+1: image

Additional note, you can simply initialize all the values to 0 and remove all of the else statements.

Edit: I realized that every if-statement is always true. image All the checks are within the boundaries of the loop. So, none of the if-statements will be false.

Are you intending to get only valid pixel indices? Because, array itself is -1 padded around the outside. I'd assume that is the intent based on the loops starting at 1 and looping to ah-1 and aw-1. Since you are doing the ==val checks, the -1 values will never equal val so it'll always give 0. Meaning, none of the if-statements are necessary anyway.

Edit 2: There is a check in pathscan (which arr in pathscan is the same array you make in layering), where it checks to see if arr[j][i] != 0. This will always be true as arr[j][i] will never be set to 0 (based on the layers assignment code in layering). The valid values are only 1-15. image

jankovicsandras commented 8 years ago
  1. Yes, you're right, the i j range checking if-statements in layering() are unnecessary, I've experimented with different padding earlier and forgot to remove them. I'll fix this.
  2. No, arr[j][i] can be 0, so this check is necessary. It's true, that the assignments in layering() are always positive, e.g. layers[val][j ][i ] = ... and the others will always be > 0, but we are not assigning values to all j i on all layers only on some layers. We're looping through all j and i, but not val in layers[val][j ][i ]. So layers[palettecolor][y][x] can be 0 (unchanged, initial default value for int in java) after layering() and arr[j][i] in pathscan() means something like layers[thisfixedcolor][y][x].

There's some info about "Edge node types" in the comments, but I'll try to be more clear: Think about arr[j][i] as it is at a corner point of a (square) pixel. So it has 4 neighbor pixels:

-------
|P1|P2|
|--X--|
|P3|P4|
-------

And the value of arr[j][i] is not a color, but a 4 bit number. Each bit means whether a neighbor pixel's color equals "thisfixedcolor" (batchpathscan() loops through the colors, so pathscan() is working with a fixed color). It's possible that none of the neighbors has "thisfixedcolor", so the value will be 0.

The dots between the squares represent arr[j][i], and the bottom right example has no yellow neighbor, its value is 0.

alt Edge node types

MiYanni commented 8 years ago

Awesome! Thank you for the info! This was my attempt at deciphering the code. :stuck_out_tongue: image

I've been using your process overview to determine how to name/identify components of the application. Basically, converting nested arrays of values into objects and concepts. It takes time, but it isn't too difficult. I've made my codebase deterministic by using a 256-bit color palette so I can test my changes in a reliable way.

What I plan on doing is finishing the type conversion and refactoring. Then, I'll start parallelizing a lot of the processing which should end up running quite fast. In some cases, I might have to use chunking or multi-buffering so that the information can be processed and updated appropriately. I'm not at that point yet, so I'll see what approaches I can take when I get there.

Thank you so much with your help! :+1:

jankovicsandras commented 8 years ago

Fixed: The unnecessary if statements are removed from layering() in v1.1.2.