Merge branch...

Merge branch '131-invalidvalue-roi-out-of-limits-when-applying-rotation-with-an-image_roi-already-defined' into 'master'

Resolve ""InvalidValue: Roi out of limits" when applying rotation with an image_roi already defined"

Closes #131

See merge request !179

common/include/lima/SizeUtils.h

@@ -296,6 +296,9 @@ class LIMACORE_API Size
 	static Point checkValid(const Point& p);
 
 	Point m_xy;
+	
+	friend bool operator ==(const Size& s1, const Size& s2);
+	friend bool operator !=(const Size& s1, const Size& s2);
 };
 
 inline bool Size::isValidCoord(int i)
@@ -310,6 +313,16 @@ inline Point Size::checkValid(const Point& p)
 	return p;
 }
 
+inline bool operator ==(const Size& s1, const Size& s2)
+{
+	return (s1.m_xy == s2.m_xy);
+}
+
+inline bool operator !=(const Size& s1, const Size& s2)
+{
+	return !(s1 == s2);
+}
+
 LIMACORE_API std::ostream& operator <<(std::ostream& os, const Size& s);
 
 
@@ -351,7 +364,8 @@ class LIMACORE_API Bin
 
 	Bin& operator /=(const Point& p)
 	{ m_xy = checkValid(m_xy / p); return *this; }
-	void swap()
+	
+	void swapDimensions()
 	{ m_xy.swap(); }
  private:
 	static bool isValidCoord(int i);

common/sip/SizeUtils.sip

@@ -61,6 +61,14 @@ class Point
 
 };
 
+Point operator +(const Point& p1, const Point& p2);
+Point operator -(const Point& p1, const Point& p2);
+Point operator *(const Point& p1, const Point& p2);
+Point operator /(const Point& p1, const Point& p2);
+Point operator %(const Point& p1, const Point& p2);
+bool operator ==(const Point& p1, const Point& p2);
+bool operator !=(const Point& p1, const Point& p2);
+
 enum XBorder {
 	Left        = 0,
 	Right       = 1,
@@ -142,6 +150,9 @@ using namespace lima;
 %End
 };
 
+bool operator ==(const Size& s1, const Size& s2);
+bool operator !=(const Size& s1, const Size& s2);
+
 class Bin
 {
 %TypeHeaderCode
@@ -167,7 +178,8 @@ using namespace lima;
 	Bin& operator *=(const Point& p);
 
 	Bin& operator /=(const Point& p);
-	void swap();
+
+	void swapDimensions();
 
         SIP_PYOBJECT __repr__() const;
 %MethodCode
@@ -398,6 +410,9 @@ using namespace lima;
 %End
 };
 
+bool operator ==(const FrameDim& f1, const FrameDim& f2);
+bool operator !=(const FrameDim& f1, const FrameDim& f2);
+
 class ArcRoi
 {
 %TypeHeaderCode

control/include/lima/CtImage.h

@@ -41,8 +41,8 @@ class LIMACORE_API CtSwBinRoiFlip {
     public:
 	friend std::ostream& operator<<(std::ostream &os,const CtSwBinRoiFlip &binroi);
 
-	CtSwBinRoiFlip(Size& size);
-	CtSwBinRoiFlip(Size& size, const Bin& bin, const Roi& roi,
+	CtSwBinRoiFlip(const Size& size);
+	CtSwBinRoiFlip(const Size& size, const Bin& bin, const Roi& roi,
 		       const Flip& flip, RotationMode rotation);
 	~CtSwBinRoiFlip();
 
@@ -70,7 +70,7 @@ class LIMACORE_API CtSwBinRoiFlip {
 	Size		m_max_size;
 	mutable Size	m_size;
 	Bin		m_bin;
-	Roi		m_roi, m_max_roi;
+	Roi		m_roi;
 	Flip    	m_flip;
 	RotationMode 	m_rotation;
 };
@@ -217,7 +217,6 @@ inline std::ostream& operator<<(std::ostream& os,const CtSwBinRoiFlip &binroi)
 	   << "m_size=" << binroi.m_size << ", "
 	   << "m_bin=" << binroi.m_bin << ", "
 	   << "m_roi=" << binroi.m_roi << ", "
-	   << "m_max_roi=" << binroi.m_max_roi << ","
 	   << "m_flip=" << binroi.m_flip
 	   << ">";
 	return os;

control/src/CtImage.cpp

@@ -28,27 +28,28 @@ using namespace lima;
 
 static const Bin Bin_1x1(1, 1);
 
-#define SWAP_DIM_IF_ROTATION(dimStruct) \
-  RotationMode aRotationMode;		\
-  getRotation(aRotationMode);		\
-  if(aRotationMode == Rotation_90 ||	\
-     aRotationMode == Rotation_270)	\
-    dimStruct.swapDimensions();
+template <typename T>
+T SwapDimIfRotated(RotationMode rotation, const T& size)
+{
+	T res = size;
+	if(rotation == Rotation_90 || rotation == Rotation_270)
+		res.swapDimensions();
+	return res;
+}
 
 // ----------------------------------------------------------------------------
 // CLASS CtSwBinRoiFlip
 // ----------------------------------------------------------------------------
-CtSwBinRoiFlip::CtSwBinRoiFlip(Size& size) :
+CtSwBinRoiFlip::CtSwBinRoiFlip(const Size& size) :
   m_rotation(Rotation_0)
 {
 	DEB_CONSTRUCTOR();
 	DEB_PARAM() << DEB_VAR1(size);
 
 	m_max_size= size;
-	m_max_roi= Roi(Point(0,0), m_max_size);
 }
 
-CtSwBinRoiFlip::CtSwBinRoiFlip(Size& size, const Bin& bin, const Roi& roi,
+CtSwBinRoiFlip::CtSwBinRoiFlip(const Size& size, const Bin& bin, const Roi& roi,
                                const Flip& flip, RotationMode rotation) :
   m_rotation(Rotation_0)
 {
@@ -56,7 +57,6 @@ CtSwBinRoiFlip::CtSwBinRoiFlip(Size& size, const Bin& bin, const Roi& roi,
 	DEB_PARAM() << DEB_VAR5(size, bin, roi, flip, rotation);
 
 	m_max_size= size;
-	m_max_roi= Roi(Point(0,0), m_max_size);
 
 	setBin(bin);
 	if (!roi.isEmpty())
@@ -76,16 +76,19 @@ void CtSwBinRoiFlip::setMaxSize(Size& size)
 	DEB_PARAM() << DEB_VAR1(size);
 
 	m_max_size= size;
-	m_max_roi.setSize(m_max_size / m_bin);
+
+	// Comppute the full ROI according to the current rotation and binning
+	Roi max_roi(Point(0,0), Size(SwapDimIfRotated(m_rotation, m_max_size) / m_bin));
+	DEB_PARAM() << DEB_VAR1(max_roi);
 
 	if (!m_roi.isEmpty()) {
 		Point roi_tl = m_roi.getTopLeft();
 		Point roi_br = m_roi.getBottomRight();
-		if (!m_max_roi.containsPoint(roi_tl)) {
+		if (!max_roi.containsPoint(roi_tl)) {
 			m_roi.reset();
 		}
-		else if (!m_max_roi.containsPoint(roi_br)) {
-			m_roi = Roi(roi_tl, m_max_roi.getBottomRight());
+		else if (!max_roi.containsPoint(roi_br)) {
+			m_roi = Roi(roi_tl, max_roi.getBottomRight());
 		}
 	}		
 }
@@ -100,7 +103,6 @@ void CtSwBinRoiFlip::setBin(const Bin& bin)
 		m_bin= bin;
 		if (!m_bin.isOne())
 			m_roi= m_roi.getBinned(m_bin);
-		m_max_roi.setSize(m_max_size / m_bin);
 	}
 }
 
@@ -109,10 +111,13 @@ void CtSwBinRoiFlip::setRoi(const Roi& roi)
 	DEB_MEMBER_FUNCT();
 	DEB_PARAM() << DEB_VAR1(roi);
 
+	Roi max_roi(Point(0,0), Size(SwapDimIfRotated(m_rotation, m_max_size) / m_bin));
+	DEB_TRACE() << DEB_VAR1(max_roi);
+
 	if (roi.isEmpty())
 		THROW_CTL_ERROR(InvalidValue) << "Software roi is empty";
-	if (!m_max_roi.containsRoi(roi))
-	  THROW_CTL_ERROR(InvalidValue) << "Roi out of limits" << DEB_VAR2(m_max_roi,roi);
+	if (!max_roi.containsRoi(roi))
+	  THROW_CTL_ERROR(InvalidValue) << "Roi out of limits " << DEB_VAR2(max_roi,roi);
 	m_roi= roi;
 }
 
@@ -139,7 +144,7 @@ const Size& CtSwBinRoiFlip::getSize() const
 	if (m_roi.isEmpty())
 		m_size= m_max_size / m_bin;
 	else	
-		m_size= Size(m_roi.getSize());
+		m_size= m_roi.getSize();
 	
 	DEB_RETURN() << DEB_VAR1(m_size);
 
@@ -557,6 +562,8 @@ CtImage::CtImage(HwInterface *hw,CtControl &ct)
 
 	m_hw_det->getMaxImageSize(m_max_size);
 	m_hw_det->getCurrImageType(m_img_type);
+	DEB_TRACE() << DEB_VAR2(m_max_size, m_img_type);
+	
 	m_next_image_type = m_img_type;
 
 	m_sw= new CtSwBinRoiFlip(m_max_size);
@@ -594,7 +601,13 @@ void CtImage::getMaxImageSize(Size& size) const
 
 	size= m_max_size;
 
-	SWAP_DIM_IF_ROTATION(size);
+	RotationMode aRotationMode;
+	getRotation(aRotationMode);
+	Bin bin;
+	getBin(bin);
+	if(aRotationMode == Rotation_90 || aRotationMode == Rotation_270)
+		size.swapDimensions();
+	size /= bin;
 
 	DEB_RETURN() << DEB_VAR1(size);
 }
@@ -636,8 +649,6 @@ void CtImage::getImageDim(FrameDim& dim) const
 	ImageType imageType = mode == Accumulation ? Bpp32S : m_img_type;
 	dim= FrameDim(m_sw->getSize(), imageType);
 
-	SWAP_DIM_IF_ROTATION(dim);
-
 	DEB_RETURN() << DEB_VAR1(dim);
 }
 
@@ -862,9 +873,15 @@ void CtImage::setFlip(Flip &flip)
   RotationMode currentRotation;
   getRotation(currentRotation);
 
-  const Size& max_roi_size = m_hw->getMaxRoiSize();
+  Size hw_max_roi_size = m_hw->getMaxRoiSize();
+  Size max_roi_size = SwapDimIfRotated(currentRotation, Size(hw_max_roi_size / m_sw->getBin()));
+  DEB_TRACE() << DEB_VAR3(currentRoi, max_roi_size, hw_max_roi_size);
+  
   currentRoi = currentRoi.getUnrotated(currentRotation,max_roi_size);
+  DEB_TRACE() << "Unrotated " << DEB_VAR1(currentRoi);
+  
   currentRoi = currentRoi.getFlipped(currentFlip,max_roi_size);
+  DEB_TRACE() << "Unflipped " << DEB_VAR1(currentRoi);
 
   if(!flip.x && ! flip.y)
     _resetFlip();
@@ -902,8 +919,12 @@ void CtImage::setRotation(RotationMode rotation)
   Bin currentBin;
   getBin(currentBin);
 
-  const Size& max_roi_size = m_hw->getMaxRoiSize();
+  Size hw_max_roi_size = m_hw->getMaxRoiSize();
+  Size max_roi_size = SwapDimIfRotated(currentRotation, Size(hw_max_roi_size / m_sw->getBin()));
+  DEB_TRACE() << DEB_VAR3(currentRoi, max_roi_size, hw_max_roi_size);
+  
   currentRoi = currentRoi.getUnrotated(currentRotation,max_roi_size);
+  DEB_TRACE() << "Unrotated " << DEB_VAR1(currentRoi);
 
   m_sw->setRotation(rotation);
 

control/test/test_ctimage.py

+from Lima import Core, Simulator
+
+Core.DebParams.setTypeFlags(Core.DebParams.AllFlags)
+
+frame_dim = Core.FrameDim(Core.Size(800, 600), Core.Bpp32F)
+
+cam = Simulator.Camera()
+getter = cam.getFrameGetter() 
+getter.setFrameDim(frame_dim)
+assert getter.getFrameDim() == frame_dim
+
+hwint = Simulator.Interface(cam)
+control = Core.CtControl(hwint)
+
+img = control.image()
+assert img.getMaxImageSize() == Core.Size(800, 600)
+assert img.getImageDim() == frame_dim
+
+###
+img.reset()
+
+# Rotate first, define ROI, unrotate
+img.setRotation(Core.Rotation_90)
+assert img.getMaxImageSize() == Core.Size(600, 800)
+
+roi = Core.Roi(250,300,300,450)
+img.setRoi(roi)
+img.setRotation(Core.Rotation_0) # InvalidValue: Invalid corner coords
+
+###
+img.reset()
+
+# Define ROI first, rotate
+roi = Core.Roi(300,50,450,300)
+img.setRoi(roi)
+img.setRotation(Core.Rotation_90) # InvalidValue: Roi out of limits m_max_roi=<0,0>-<800x600>, roi=<250,300>-<300x450>
+
+###
+img.reset()
+
+# Define ROI first, bin
+roi = Core.Roi(300,50,450,300)
+img.setRoi(roi)
+img.setBin(Core.Bin(2, 2))
+assert img.getMaxImageSize() == Core.Size(400, 300) # Fail

control/test/test_readimage.py

+###########################################################################
+# This file is part of LImA, a Library for Image Acquisition
+#
+#  Copyright (C) : 2009-2018
+#  European Synchrotron Radiation Facility
+#  BP 220, Grenoble 38043
+#  FRANCE
+# 
+#  Contact: lima@esrf.fr
+# 
+#  This is free software; you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation; either version 3 of the License, or
+#  (at your option) any later version.
+# 
+#  This software is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#  GNU General Public License for more details.
+# 
+#  You should have received a copy of the GNU General Public License
+#  along with this program; if not, see <http://www.gnu.org/licenses/>.
+############################################################################
+
+from Lima import Core, Simulator
+
+Core.DebParams.setTypeFlags(Core.DebParams.AllFlags)
+
+cam = Simulator.Camera()
+
+frame_dim = Core.FrameDim(Core.Size(800, 600), Core.Bpp32)
+cam = Simulator.Camera()
+getter = cam.getFrameGetter() 
+getter.setFrameDim(frame_dim)
+
+hwint = Simulator.Interface(cam)
+control = Core.CtControl(hwint)
+
+acq = control.acquisition()
+acq.setAcqNbFrames(1)
+
+img = control.image()
+roi = Core.Roi(300,50,450,300)
+img.setRoi(roi)
+# img.setBin(Core.Bin(2, 2))
+img.setRotation(Core.Rotation_90)
+
+control.prepareAcq();
+control.startAcq();
+
+while control.getStatus() == Core.AcqRunning:
+    time.sleep(0.001)
+
+control.stopAcq();
+
+frame = control.ReadImage(0)

control/test/test_roi_binrotflip.py

+import time
+from Lima import Core, Simulator
+
+#Core.DebParams.setTypeFlags(Core.DebParams.AllFlags)
+
+cam = Simulator.Camera()
+
+#frame_dim = Core.FrameDim(Core.Size(800, 600), Core.Bpp32)
+frame_dim = Core.FrameDim(Core.Size(600, 600), Core.Bpp32)
+cam = Simulator.Camera()
+getter = cam.getFrameGetter()
+getter.setFrameDim(frame_dim)
+
+hwint = Simulator.Interface(cam)
+control = Core.CtControl(hwint)
+
+acq = control.acquisition()
+acq.setAcqNbFrames(1)
+acq.setAcqExpoTime(0.001)
+
+img = control.image()
+#roi = Core.Roi(300,50,450,300)
+roi = Core.Roi(160, 60, 120, 200)
+img.setRoi(roi)
+
+rots = [Core.Rotation_0, Core.Rotation_90, Core.Rotation_180, Core.Rotation_270]
+binnings = [Core.Bin(1,1), Core.Bin(1,2), Core.Bin(2,1), Core.Bin(2,2)]
+flips = [Core.Flip(False, False), Core.Flip(False, True), Core.Flip(True, False), Core.Flip(True, True)]
+
+# Python function to print permutations of a given list (from SO)
+def permutation(lst):
+    if len(lst) == 0:
+        return []
+
+    if len(lst) == 1:
+        return [lst]
+
+    l = []
+    for i in range(len(lst)):
+       m = lst[i]
+
+       remLst = lst[:i] + lst[i+1:]
+
+       for p in permutation(remLst):
+           l.append([m] + p)
+    return l
+
+permutations = permutation(['b', 'f', 'r'])
+
+# For every permutation of every Bin Rot Flip parameters applied in any order
+for rot in rots:
+    for bin in binnings:
+        for flip in flips:
+            for perm in permutations:
+
+                try:
+                    for op in perm:
+                        if op == 'f':
+                            img.setFlip(flip)
+                        elif op == 'b':
+                            img.setBin(bin)
+                        elif op == 'r':
+                            img.setRotation(rot)
+
+                    #print(perm, flip, rot, bin, img.getRoi(), " - OK")
+
+                except Exception:
+                    print(perm, flip, rot, bin, img.getRoi(), " - FAILED")
+
+img.resetBin()
+img.resetFlip()
+img.resetRotation()
+
+assert img.getRoi() == roi