peterbraden / node-opencv

OpenCV Bindings for node.js
MIT License
4.38k stars 857 forks source link

Refcount #588

Closed btsimonh closed 6 years ago

btsimonh commented 6 years ago

Adds Matrix.getrefCount() Only really useful for debugging, but not harmful consider addref() to be detrimental - dangerous if someone used it by mistake.

codecov-io commented 6 years ago

Codecov Report

Merging #588 into master will increase coverage by 0.1%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #588     +/-   ##
=========================================
+ Coverage   56.65%   56.75%   +0.1%     
=========================================
  Files          33       33             
  Lines        4037     4047     +10     
  Branches       30       30             
=========================================
+ Hits         2287     2297     +10     
  Misses       1750     1750
Impacted Files Coverage Δ
src/Matrix.h 100% <ø> (ø) :arrow_up:
src/Matrix.cc 55.68% <100%> (+0.23%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 649df8f...f4d0459. Read the comment docs.

btsimonh commented 6 years ago

Please don't merge! Further investigation shows that mat.release() decrements the ref count, and then ditches the link to the data, regardless of the ref count. So addref really does not actually do anything we could want? (I think). reading the ref count is interesting from a debug perspective.... but more thought required.

btsimonh commented 6 years ago

Peter - I leave this one to you to decide. It's not harmful in itself, but only useful if debugging.

btsimonh commented 6 years ago

good spot. done..

peterbraden commented 6 years ago

Thanks!