Skip to content

Cocoa/highgui: replace with @autoreleasepool blocks#26660

Merged
asmorkalov merged 1 commit intoopencv:4.xfrom
NekoAsakura:4.x
Jan 24, 2025
Merged

Cocoa/highgui: replace with @autoreleasepool blocks#26660
asmorkalov merged 1 commit intoopencv:4.xfrom
NekoAsakura:4.x

Conversation

@NekoAsakura
Copy link
Copy Markdown
Contributor

  1. According to Apple Developer Documentation, @autorelease block is the preferred method over using an instance of NSAutoreleasePool directly. This is also discussed in Cocoa/highgui: fix leak in cvGetWindowRect_COCOA #26625 (comment).

  2. Starting from commit 4ec4cf6, highgui_cocoa.mm contained numerous debug lines that had been commented out, such as:

    static CVWindow *cvGetWindow(const char *name) {
    //cout << "cvGetWindow" << endl;
    NSAutoreleasePool* localpool = [[NSAutoreleasePool alloc] init];
    NSString *cvname = [NSString stringWithFormat:@"%s", name];
    CVWindow* retval = (CVWindow*) [windows valueForKey:cvname] ;
    //cout << "retain count: " << [retval retainCount] << endl;
    //retval = [retval retain];
    //cout << "retain count: " << [retval retainCount] << endl;
    [localpool drain];
    //cout << "retain count: " << [retval retainCount] << endl;
    return retval;
    }

    Such extraneous comments should be removed once debugging is complete and should not remain in the release environment. A previous commit fb34f36 had partially cleaned up these comments. This PR made a thorough review and cleaned up the remaining extraneous comments.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@asmorkalov asmorkalov added platform: ios/osx cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) labels Dec 23, 2024
@asmorkalov asmorkalov added this to the 4.12.0 milestone Jan 4, 2025
@NekoAsakura
Copy link
Copy Markdown
Contributor Author

Thank you for your time reviewing! I have made the suggested changes in the latest commit.

@asmorkalov
Copy link
Copy Markdown
Contributor

@VadimLevin please take a look again.

@NekoAsakura
Copy link
Copy Markdown
Contributor Author

Oops, some tests have failed. It seems this is due to accidental variable shadowing. I will correct it asap.

@opencv-alalek
Copy link
Copy Markdown
Contributor

CI logs:

modules/highgui/src/window_cocoa.mm:275: trailing whitespace.
+    
modules/highgui/src/window_cocoa.mm:799: trailing whitespace.
+        

https://github.com/opencv/opencv/wiki/How_to_contribute#q3-i-was-asked-to-remove-whitespace-issues-how-can-i-do-that

@NekoAsakura
Copy link
Copy Markdown
Contributor Author

Thank you :) it should have been fixed.

@asmorkalov asmorkalov merged commit a3e95ec into opencv:4.x Jan 24, 2025
@asmorkalov asmorkalov mentioned this pull request Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Code cleanup (e.g, drop legacy C-API, legacy unmaintained code) platform: ios/osx

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants