How to add/subtract/multiply/divide by Scalar?

[b005t3r]

Is it even possible to use functions like add() or multiply() with Scalars (e.g. to multiply each element of a Mat by a given Scalar value)? I don't see a way of doing that.

[saudet]

Wrapping them in a Mat should work. Let me know if it doesn't work though.

[b005t3r]

No, it doesn't work, unless I'm doing something wrong:

    private void applyColors(Mat bgra, Scalar leftColor, Scalar midColor, Scalar rightColor) {
        Mat leftRegion  = new Mat(bgra, new Rect(0, 0, 175, bgra.rows()));
        Mat midRegion   = new Mat(bgra, new Rect(175, 0, 175, bgra.rows()));
        Mat rightRegion = new Mat(bgra, new Rect(350, 0, 175, bgra.rows()));
        
        multiply(leftRegion, new Mat(leftColor), leftRegion, 1.0 / 255.0, bgra.type());
        multiply(midRegion, new Mat(midColor), midRegion, 1.0 / 255.0, bgra.type());
        multiply(rightRegion, new Mat(rightColor), rightRegion, 1.0 / 255.0, bgra.type());
    }

Exception:

Exception in thread "main" java.lang.RuntimeException: OpenCV(4.3.0) /Users/travis/build/bytedeco/javacpp-presets/opencv/cppbuild/macosx-x86_64/opencv-4.3.0/modules/core/src/arithm.cpp:671: error: (-215:Assertion failed) type2 == CV_64F && (sz2.height == 1 || sz2.height == 4) in function 'arithm_op'

[b005t3r]

This works, but if I understand it correctly, it allocates three additional Mats:

    private void applyColors(Mat bgra, Scalar leftColor, Scalar midColor, Scalar rightColor) {
        Mat leftRegion  = new Mat(bgra, new Rect(0, 0, 175, bgra.rows()));
        Mat midRegion   = new Mat(bgra, new Rect(175, 0, 175, bgra.rows()));
        Mat rightRegion = new Mat(bgra, new Rect(350, 0, 175, bgra.rows()));
        
        Mat leftMul     = new Mat(leftRegion.rows(), leftRegion.cols(), leftRegion.type(), leftColor);
        Mat midMul      = new Mat(midRegion.rows(), midRegion.cols(), midRegion.type(), midColor);
        Mat rightMul    = new Mat(rightRegion.rows(), rightRegion.cols(), rightRegion.type(), rightColor);
        
        multiply(leftRegion, leftMul, leftRegion, 1.0 / 255.0, bgra.type());
        multiply(midRegion, midMul, midRegion, 1.0 / 255.0, bgra.type());
        multiply(rightRegion, rightMul, rightRegion, 1.0 / 255.0, bgra.type());
    }

[saudet]

Ah, I see, that's a regression from commit 2874998. new Mat(Scalar) is no longer calling new Mat(1, 1, ...).

@HGuillemet Could you please fix this?

[HGuillemet]

I don't think there is a regression new Mat(Scalar) still works as it used to do:

System.err.println(new Mat(new Scalar(1,2,3,4)));

Gives org.bytedeco.opencv.opencv_core.Mat[width=1,height=1,depth=64,channels=4]

But it seems that when a function taking an InputArray receives a Mat representing a scalar, it oddly expects a Mat with 4 rows and 1 channel.

In the example above, replacing new Mat(leftColor) by new Mat(4, 1, CV_64FC1, leftColor) should work.

We could change the Mat constructors taking a Scalar and return a multi-row Mat instead of a multi-channel Mat.
Or add a asInputArray() method to preserve the multi-channel constructor.

[saudet]

@HGuillemet I see, I had not confirmed before replying, sorry. So it looks like that function wants a vector. Thanks for checking!

@b005t3r Does it work if you give it a vector like that?

[b005t3r]

I think I like the asInputArray() method added to Scalar the most - makes it more clear to me.

Thanks, guys!

[HGuillemet]

Before we implement such method, can you try to replace the Mat creation in your code with something like new Mat(4, 1, CV_64FC1, leftColor) and tell us if it works ?

[b005t3r]

OK, I tested that and it doesn't crash, but it also doesn't work - there's no color change in the resulting mat.

[HGuillemet]

I see: Mat(4, 1, CV_64FC1, leftColor) calls the Mat(rows, cols, type, Scalar) constructor that fills a 4x1 matrix with the scalar, which is not what we want. Mat(4, 1, CV_64FC1, leftColor, false) may work, but could also produce a SIGSEGV.

And Mat(4, 1, CV_64FC1, leftColor, true) does not work for another reason:
a Scalar has a sizeof of 8 and a limit of 1 and Mat.data().put(Scalar) would then only copy the first component.
@saudet, what do you think about this ? Shouldn't a Scalar have a limit and capacity of 4 ?

[saudet]

Each Scalar has 4 values, but for 1 of those, the size is 1, just like it is in a C/C++ array.

[HGuillemet]

So sizeof should be 32 ? But since it extends DoublePointer, that would probably cause issues.

[saudet]

Yes, that's what it should be returning already...

[HGuillemet]

I does not, because of DoublePointer inheritance. We should drop this inheritance and replace in with some DoubleVector.

Can you think of a way to enable in Java the polymorphism that the proxy classes InputArray and OutputArray allows in the C++ API ? Can we add some abstract class or interface shared by Scalar, Mat, ... ? Or add more overrides to the methods accepting InputArray ?

[saudet]

I does not, because of DoublePointer inheritance. We should drop this inheritance and replace in with some DoubleVector.

Ah, right, I had hard coded those methods of sizeof(). I'm not sure if it's worth doing something about this though because Loader.sizeof(Scalar.class) returns the correct value. What could we get fixed this way?

Can you think of a way to enable in Java the polymorphism that the proxy classes InputArray and OutputArray allows in the C++ API ? Can we add some abstract class or interface shared by Scalar, Mat, ... ? Or add more overrides to the methods accepting InputArray ?

We could map InputArray and OutputArray but we would have to create these objects everywhere manually because there is no way to create objects implicitly in Java. We could instead just add overloads taking Scalar and what not where it's needed, sure, but there is no automated way to do that because OpenCV only provides that information dynamically at runtime.

[saudet]

I see, it's because new Mat(4, 1, CV_64FC1, aScalar) ends up calling Pointer.sizeof() that it causes problems.
I've fixed that in the commit bytedeco/javacpp@f3494a7.

@b005t3r Please give it a try with the snapshots: http://bytedeco.org/builds/
Thanks for your help debugging this!

[saudet]

It looks like Mat.setTo() and inRange() also accept vectors in the case of scalars these days, so it's probably ok to change new Mat(aScalar) to return new Mat(4, 1, CV_64FC1, aScalar), and similarly for others. @HGuillemet What do you think?

[HGuillemet]

Ah, right, I had hard coded those methods of sizeof(). I'm not sure if it's worth doing something about this though because Loader.sizeof(Scalar.class) returns the correct value. What could we get fixed this way?

I'm not convinced by your commit. You do fix the returned value of sizeof, which solves the offset/length calculations made in methods like Pointer.put(Pointer), but you end up with an hybrid class that is basically an array of elements whose size is not the size returned by sizeof(). Scalar instances are arrays of Scalar and not arrays of doubles, so IMO they shouldn't inherit from DoublePointer.
This remains a bit theoretical because I'm not sure there are many situations where we can have Scalar pointing to more than 1 scalar. But if ever we do :

Scalar twoScalars = new Scalar(new DoublePointer(8));
twoScalars.put(1, 2, 3, 4, 5, 6, 7, 8);

For instance twoScalars.position(1) returns a DoublePointer, not a Scalar, and it's unclear what it points to.
BTW, Scalar.toString do support arrays of Scalar, but doesn't return the right string because of its use of position;
(1.0, 2.0, 3.0, 4.0) (2.0, 3.0, 4.0, 5.0) (3.0, 4.0, 5.0, 6.0) (4.0, 5.0, 6.0, 7.0) (5.0, 6.0, 7.0, 8.0) (6.0, 7.0, 8.0, 1.0) (7.0, 8.0, 1.0, 0.0) (8.0, 1.0, 0.0, 0.0)

[HGuillemet]

We could map InputArray and OutputArray but we would have to create these objects everywhere manually because there is no way to create objects implicitly in Java. We could instead just add overloads taking Scalar and what not where it's needed, sure, but there is no automated way to do that because OpenCV only provides that information dynamically at runtime.

Wouldn't it work if we define an empty "tag" interface InputArray, have Mat, Scalar and possible other classes implement this interface, and map the C++ InputArray to this interface ? That would match quite well the C++ interface.

EDIT: I guess it won't, since the C++ method will treat arguments received this way as instances of the C++ class InputArray while they are not, and it won't create the proxy instances.

[HGuillemet]

It looks like Mat.setTo() and inRange() also accept vectors in the case of scalars these days, so it's probably ok to change new Mat(aScalar) to return new Mat(4, 1, CV_64FC1, aScalar), and similarly for others. @HGuillemet What do you think?

If there is no easy way to pass directly the scalar to methods taking InputArray, I think it's the way to go, adding the copyData argument or the wrong constructor would be called as said above. Also I'm not sure if we must keep the default value of copyData to false in the case of Scalar.

[saudet]

I'm not convinced by your commit. You do fix the returned value of sizeof, which solves the offset/length calculations made in methods like Pointer.put(Pointer), but you end up with an hybrid class that is basically an array of elements whose size is not the size returned by sizeof(). Scalar instances are arrays of Scalar and not arrays of doubles, so IMO they shouldn't inherit from DoublePointer.

Sure, of course, it's not ideal. Ideally, we would be mapping class templates like cv::Vec and cv::Matx. Feel free to give it a try! :)

For instance twoScalars.position(1) returns a DoublePointer, not a Scalar, and it's unclear what it points to.

Yeah, there's a missing override of position() there that went away when issue bytedeco/javacv#1224 got fixed. I haven't encountered anyone yet that needs an array of cv::Scalar though.

BTW, Scalar.toString do support arrays of Scalar, but doesn't return the right string because of its use of position;
(1.0, 2.0, 3.0, 4.0) (2.0, 3.0, 4.0, 5.0) (3.0, 4.0, 5.0, 6.0) (4.0, 5.0, 6.0, 7.0) (5.0, 6.0, 7.0, 8.0) (6.0, 7.0, 8.0, 1.0) (7.0, 8.0, 1.0, 0.0) (8.0, 1.0, 0.0, 0.0)

Yes, OpenCV changed how cv::Scalar was defined at some point, and the get() function got moved to cv::Matx...

Wouldn't it work if we define an empty "tag" interface InputArray, have Mat, Scalar and possible other classes implement this interface, and map the C++ InputArray to this interface ? That would match quite well the C++ interface.

EDIT: I guess it won't, since the C++ method will treat arguments received this way as instances of the C++ class InputArray while they are not, and it won't create the proxy instances.

Right, we'd need to create another API on top of the C++ API.

If there is no easy way to pass directly the scalar to methods taking InputArray, I think it's the way to go, adding the copyData argument or the wrong constructor would be called as said above. Also I'm not sure if we must keep the default value of copyData to false in the case of Scalar.

It'd be confusing to have the default value of that flag depend on the source of data. I think it's fine with false by default because, in this case, we are using Mat only as a temporary wrapper for whatever we can't pass directly.

[HGuillemet]

I'm not convinced by your commit. You do fix the returned value of sizeof, which solves the offset/length calculations made in methods like Pointer.put(Pointer), but you end up with an hybrid class that is basically an array of elements whose size is not the size returned by sizeof(). Scalar instances are arrays of Scalar and not arrays of doubles, so IMO they shouldn't inherit from DoublePointer.

Sure, of course, it's not ideal. Ideally, we would be mapping class templates like cv::Vec and cv::Matx. Feel free to give it a try! :)

What about simply inheriting from Pointer and not DoublePointer ?If there are methods of DoublePointer that users may use, we can implement them in AbstractScalar. We won't need to change sizeof in DoublePointer, nor add override for position, limit... I can try this first if you think it's worth.

Also I'm not sure if we must keep the default value of copyData to false in the case of Scalar.

It'd be confusing to have the default value of that flag depend on the source of data. I think it's fine with false by default because, in this case, we are using Mat only as a temporary wrapper for whatever we can't pass directly.

Ok.

[saudet]

I'm not convinced by your commit. You do fix the returned value of sizeof, which solves the offset/length calculations made in methods like Pointer.put(Pointer), but you end up with an hybrid class that is basically an array of elements whose size is not the size returned by sizeof(). Scalar instances are arrays of Scalar and not arrays of doubles, so IMO they shouldn't inherit from DoublePointer.

Sure, of course, it's not ideal. Ideally, we would be mapping class templates like cv::Vec and cv::Matx. Feel free to give it a try! :)

What about simply inheriting from Pointer and not DoublePointer ?If there are methods of DoublePointer that users may use, we can implement them in AbstractScalar. We won't need to change sizeof in DoublePointer, nor add override for position, limit... I can try this first if you think it's worth.

We need setters and getters. Pointer doesn't have those and we can't put them in AbstractScalar either because there is no corresponding C++ class. Sure, we could hack something, but if you really want to do that, it's probably easier to just map the instances of cv::Vec and cv::Matx anyway.

Also I'm not sure if we must keep the default value of copyData to false in the case of Scalar.

It'd be confusing to have the default value of that flag depend on the source of data. I think it's fine with false by default because, in this case, we are using Mat only as a temporary wrapper for whatever we can't pass directly.

Ok.

Actually, while we're at it, we should also update the constructors taking Java arrays to create column vectors instead of row vectors, to make it consistent with (newish) C++ constructors such as Mat(std::vector) and InputArray(std::vector). That way it would also allow us to do something simple like new Mat(1, 2, 3, 4) instead of worrying about instances of Scalar and what not.

[saudet]

Hum, cv::Mat(cv::Scalar(1)) returns a matrix with rows=4, cols=1, and channels=1, while cv::Mat(std::vector<cv::Scalar>(2)) returns one with rows=2, cols=1, and channels=4. This is annoying... OpenCV isn't very consistent here.

[saudet]

I've changed the constructors for Mat in commit 52df2ef to get column vectors when passing arrays, avoiding the need to bother with Scalar at all. We should be able to pass new Mat(1.0, 2.0, 3.0, 4.0) to the same effect everywhere. It's still not going to work for new Mat(aScalar) because in that case we get a 1x1 matrix with 4 channels, but just for that case, we can easily enough pass the values to the array constructor, when it's required, as it's not always required. Functions like inRange() and setTo() work with pretty much anything.

[HGuillemet]

Sorry for not being responsive and available for testing things those days.
It's more consistent with the C++ API indeed if the array constructor returns a vector.
I'll attempt to map Vec and or Matx when I have enough time and report here.

[saudet]

Can we consider this "fixed"? What do you think @b005t3r?

[b005t3r]

I haven't tested the change yet and I'm not sure when I will, so please mark it as fixed if you think it's fixed enough :)

[saudet]

Sounds good, please reopen if you encounter with any issues with that the next time. Thanks for reporting!