CenterForDigitalHumanities / SpectralRTI_Toolkit

Process Spectral RTI Images in ImageJ
GNU General Public License v3.0
1 stars 0 forks source link

listOfRakingDirections #33

Closed thehabes closed 6 years ago

thehabes commented 6 years ago

I am a bit confused about the iterations around this variable. It is either initiated as

for(i=0;i<listOfHemisphereCaptures.length;i++) {
    listOfRakingDirections = Array.concat(listOfRakingDirections,Dialog.getCheckbox());
}

or if no raking tasks are selected it is initiated as listOfRakingDirections = newArray(listOfHemisphereCaptures.length+1);

Later, checks are made against it like

if(xxRTIDesired){
     if (listOfRakingDirections[i+1]) {
        //make static raking file
     }
}

I have a couple questions around this.

Why is listOfRakingDirections initiated at a length of listOfHemisphereCaptures.length+1 instead of listOfHemisphereCaptures.length?

Why isn't it a check against listOfRakingDirections[i] instead of listOfRakingDirections[i+1]?

It all seems to work as is, but this seems to confuse the RTI processes as to whether just to make source file jpgs or static raking jp2's in some cases. I had to do some bug fixing around that problem and it all revolved around this variable. The falsiness of initializing the array does not work the same in Java. I have it working just as the macro does, but the code is a bit confusing and I would like to rewrite it but I need some understanding around it first.

thanneken commented 6 years ago

I don't remember exactly, but I doubt it was profound. If you have an idea to clean up the code, and it works, go for it.

My best guess as to the +1 issue is that the last member of an array of length x is x-1 because the first element in the array is 0, not 1. Or maybe it has something to do with how the array is populated; perhaps the first element in the array is not an image file?

At any rate, it should be pretty easy to test. Ask it to create a static raking. If it does the right one, great, if off by one, back to the math.

thanneken commented 6 years ago

In testing today I asked for A03, A13, G05, G13 = _02, _07, _51, _55, got _01, _06, _50, _54. Whatever the underlying reason is, this is why the macro had a +1.

thehabes commented 6 years ago

So here is the answer to the fuzziness. Something is unclean about that whole process, I am going to investigate this deeper today.

thehabes commented 6 years ago

I asked for A03, A13, G05, G13 = _02, _07, _51, _55, got _01, _06, _50, _54.

^^ Is this an error, or what you expected?

thanneken commented 6 years ago

It's an error. For each light position I select, it gives me the previous one.

thehabes commented 6 years ago

In the macro, whether Raking is desired or not, the length of this variable is 57 (for 56 captures)

In the plugin, if Raking is desired, the length is 56 In the plugin, if Raking is not desired, the length is 57

Something about the Macro made that length 57 even when there were only 56 checkboxes to choose from and it does not work the same in the plugin.

The length at these points in the Plugin need to match the length it is in the macro for this to work properly. I will really have to dig in to figure out why there are length mismatches.

thehabes commented 6 years ago

I should be able to pad the variable with an extra "false" at the beginning or end, which will give the desired effect of 57 all around, but it still makes me worried about the numbering like mentioned above.

Macro: If raking is desired (and the user had checkboxes to check)

for(i=0;i<listOfHemisphereCaptures.length;i++) {
    listOfRakingDirections = Array.concat(listOfRakingDirections,Dialog.getCheckbox());
}

if no raking tasks are selected it is initiated as listOfRakingDirections = newArray(listOfHemisphereCaptures.length+1);

In both cases the length is 57 for 56 captures.

Plugin: If raking is desired (and the user had checkboxes to check)

for(JCheckBox check : positions){
       listOfRakingDirections.add(check.isSelected());
}

which ends up length 56 for 56 captures.

if no raking tasks are selected it is initiated as

listOfRakingDirections = new ArrayList<>();
while(listOfRakingDirections.size() < listOfHemisphereCaptures.length+1) listOfRakingDirections.add(Boolean.FALSE);

which ends up with length 57 for 56 captures

thehabes commented 6 years ago

Testing shows this seems to fixed.