pose2sim icon indicating copy to clipboard operation
pose2sim copied to clipboard

Ask user during find corner at the end to validate the data

Open ANaaim opened this issue 10 months ago • 17 comments

Could be good if you test also (it work fine for me)

ANaaim avatar Apr 09 '24 07:04 ANaaim

linked to #89

ANaaim avatar Apr 09 '24 07:04 ANaaim

Thank you Alex for this pull request! It works great, however I ran into two small issues:

  • I had to explicitly add import tkinter.messagebox to the file, otherwise I ran into an error
  • There is a tiny tkinter window that is floating around, and if I try to close it it causes python to exit. Do you have it on your side?
  • And a minor additional thing: the window is not currently triggered if I type H (hidden last point) instead of if I click. If would be nice if that case was taken into account, too

image

davidpagnon avatar Apr 09 '24 22:04 davidpagnon

Last thing @ANaaim, do you think you could pop that window both when clicking the intrinsics and the extrinsics? (apologies if you already did)

davidpagnon avatar Apr 12 '24 13:04 davidpagnon

I will have a look at it on Monday.

ANaaim avatar Apr 12 '24 17:04 ANaaim

@davidpagnon (some answer in your message) Thank you Alex for this pull request! It works great, however I ran into two small issues:

  • I had to explicitly add import tkinter.messagebox to the file, otherwise I ran into an error
    • I do not understand why because when used I definitely use as the following :
msg_box = tkinter.messagebox.askquestion(
                            "Validate Calibration", "Are you satisfied with point positioning ? ", icon="warning"
                        )
  • There is a tiny tkinter window that is floating around, and if I try to close it it causes python to exit. Do you have it on your side? - Yes I will remove it. It should not be difficult.
  • And a minor additional thing: the window is not currently triggered if I type H (hidden last point) instead of if I click. If would be nice if that case was taken into account, too
    • Would it be possible to switch all this bit below in on_click instead of on_key (as it is linked to adding point it seems more logical)
        if event.key == 'h':
            # If 'h', indicates that one of the objp is not visible on image
            # Displays it in red on 3D plot
            if len(objp) != 0  and 'ax_3d' in globals():
                count = [0 if 'count' not in globals() else count+1][0]
                if 'events' not in globals():
                    # retrieve first objp_confirmed_notok and plot 3D
                    events = [event]
                    objp_confirmed_notok = objp[count]
                    ax_3d.scatter(*objp_confirmed_notok, marker='o', color='r')
                    fig_3d.canvas.draw()
                elif count == len(objp)-1:
                    # if all objp have been clicked or indicated as not visible, close all
                    objp_confirmed = np.array([[objp[count]] if 'objp_confirmed' not in globals() else objp_confirmed+[objp[count]]][0])[:-1]
                    imgp_confirmed = np.array(np.expand_dims(scat.get_offsets(), axis=1), np.float32) 
                    plt.close('all')
                    for var_to_delete in ['events', 'count', 'scat', 'fig_3d', 'ax_3d', 'objp_confirmed_notok']:
                        if var_to_delete in globals():
                            del globals()[var_to_delete]
                else:
                    # retrieve other objp_confirmed_notok and plot 3D
                    events.append(event)
                    objp_confirmed_notok = objp[count]
                    ax_3d.scatter(*objp_confirmed_notok, marker='o', color='r')
                    fig_3d.canvas.draw()
            else:
                pass

For the last bit, maybe the function on_key and on_click should be renamed menu_event and adding_point_event (or something like this)

ANaaim avatar Apr 15 '24 06:04 ANaaim

  • do not understand why because when used I definitely use as the following :
msg_box = tkinter.messagebox.askquestion(
                            "Validate Calibration", "Are you satisfied with point positioning ? ", icon="warning"
                        )

You are right, this is strange, I could have made a mistake.


For the last bit, maybe the function on_key and on_click should be renamed menu_event and adding_point_event (or something like this)

Sure, that sounds coherent! I'm all up for something clearer (as long as it still works :) )

davidpagnon avatar Apr 15 '24 07:04 davidpagnon

Work with intrinsic as well as it is using the same function.

ANaaim avatar Apr 15 '24 15:04 ANaaim

Cool, the messagebox is now closing, and it shows both for intrinsics and for extrinsics! A couple last remarks:

  • I don't quite understand why, but I do need to import tkinter.messagebox or it throws an error at me. However, I notice that the following imports are not useful anymore: import tkinter from contextlib import contextmanager,redirect_stderr,redirect_stdout from os import devnull

  • There is an unnecessary print(count) L978

  • The lines 524+ are not needed, as discussed in this issue

     ret, C, S, D, K, R, T = calibrate_intrinsics(calib_dir, intrinsics_config_dict)
    
    # calculate extrinsics
    if calculate_extrinsics:
       logging.info(f'\nCalculating extrinsic parameters...')
    

    (and L689, imgp == [] must be replaced with len(imgp) == 0).

  • I added objpoints = np.array(objpoints) L605, otherwise I've got an error when clicking intrinsic parameters:

    error: OpenCV(4.9.0) :-1: error: (-5:Bad argument) in function 'calibrateCamera'
    > Overload resolution failed:
    >  - Can't parse 'objectPoints'. Sequence item with index 0 has a wrong type
    >  - Can't parse 'objectPoints'. Sequence item with index 0 has a wrong type
    
  • Lastly, it seems like pressing 'H' does not work anymore: the hidden objpoint is removed from the list and then cv2.calibrationCamera() does not get the expected size for objpoints. I haven't looked into solving this yet.

davidpagnon avatar Apr 15 '24 23:04 davidpagnon

I tested the h button with board, it seemed to work fine. Is there different stuff associated with using a scene that could result in this behavior ?

ANaaim avatar Apr 16 '24 07:04 ANaaim

Not sure to understand where it should be put for this :

I added objpoints = np.array(objpoints) L605, otherwise I've got an error when clicking intrinsic parameters: This :

                imgp_confirmed = findCorners(img_path, intrinsics_corners_nb, objp=objp, show=show_detection_intrinsics)
                if isinstance(imgp_confirmed, np.ndarray):
                    imgpoints.append(imgp_confirmed)
                    objpoints.append(objp)

Should become this ? :

                imgp_confirmed = findCorners(img_path, intrinsics_corners_nb, objp=objp, show=show_detection_intrinsics)
                if isinstance(imgp_confirmed, np.ndarray):
                    objpoints = np.array(objpoints)
                    imgpoints.append(imgp_confirmed)
                    objpoints.append(objp)

ANaaim avatar Apr 16 '24 07:04 ANaaim

Oh yes sorry the lines have shifted:

        # calculate intrinsics
        img = cv2.imread(str(img_path))
        objpoints = np.array(objpoints) # <-- here (L602)
        ret_cam, mtx, dist, rvecs, tvecs = cv2.calibrateCamera(objpoints, imgpoints, img.shape[1::-1], 
                                    None, None, flags=(cv2.CALIB_FIX_K3 + cv2.CALIB_FIX_PRINCIPAL_POINT))

I still have that issue with 'H', do you not have it at all? The error does not happen right away, but after I've done all the clicking for all images for a given camera.

davidpagnon avatar Apr 16 '24 10:04 davidpagnon

Is the error in calibration intrinsic only ?

ANaaim avatar Apr 17 '24 06:04 ANaaim

Ithink that the problem come from the fact that during a intrinsic calibration it create a list of associated objpoint and imgpoint that might have difference in size... as a result it might not work. But for the intrinsics calibration for me it should not be possible to do a calibration without seeing the full board...

ANaaim avatar Apr 17 '24 06:04 ANaaim

line: 607 objpoints = np.array(objpoints)

Here all the objpoints element of the list should have the same dimension if we want to transform it to a np array. For me the H as sense only during the extrinsics calibration. (We should even deactivate H during intrinsics)

ANaaim avatar Apr 17 '24 06:04 ANaaim

Why should we deactivate it? If we don't have that many checkerboard images, and for one of them some corners are out of the frame (like in this one), it would be nice to keep the option of declaring that a point is not seen, wouldn't it?

image

davidpagnon avatar Apr 17 '24 07:04 davidpagnon

Ok but in that case the problem is in the following function

ret_cam, mtx, dist, rvecs, tvecs = cv2.calibrateCamera(objpoints, imgpoints, img.shape[1::-1],
                                                               None, None,
                                                               flags=(cv2.CALIB_FIX_K3 + cv2.CALIB_FIX_PRINCIPAL_POINT))

Can it take objpoints and imgpoints that are a list of numpy array that are not the same shape ? Because each associated obj_confirmed and img_confirmed are correctly associated with the same number of point.

                    imgpoints.append(imgp_confirmed)
                    objpoints.append(objp_confirmed)

ANaaim avatar Apr 17 '24 08:04 ANaaim

No, they need to be the same shape.

I would need to go back to the code to remember the details, but the rationale for the way I did it was in The way I did it was in imgp_objp_visualizer_clicker: If 'H' was typed for a certain index, imgp was not created, and the corresponding objp was removed from objpoints

davidpagnon avatar Apr 17 '24 09:04 davidpagnon