Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes & improvements to Image, including scaling & arbitrary rotation. #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

neildavis
Copy link
Contributor

This commit adds bilinear interpolation scaling and arbirary rotation methods to Image.
In the process, several issues with Image were found & fixed:

  • There was no destructor to free the canvas memory. This would cause a memory leak unless the client remembered to call close() A virtual destructor has been added to free this memory.

  • There was no explicit copy & move constructor for Image This means whenever Image was copied or passed by value a 'shallow copy' was performed resulting in both instances referencing the same shared canvas memory. This works, and several examples were depending on it, but after the addition of the destructor it caused seg faults since the memory was free()'d several times. An explicit copy constuctor for Image has been added to perform a 'deep copy' of the canvas memory. A C++11 'move' constructor has been added to avoid excessive deep copies being made when passing Image as a temporary.

  • The same logic for copy/move constructors also applies to assignment operators (operator=()) so these are added.

  • Image (and Color) are extensively passed by value within the library. This now potentially inefficient due to the deep copy. (if the move c'tor is not used) Since pass-by-reference semantics are required in these cases they are now passed by reference.

  • Pass-by-(const)-reference changes also required many methods on Image and Color that did not modify their members to be declared const as well as all Color preset constants.

  • New c'tors and getters added to Color to support RGBA access as required by the new scaling method on Image Also tidied up c'tors to use common helper methods.

  • Used new/delete instead of malloc/free for heap memory use in Image Just coz it's more conventional in C++

This commit adds bilinear interpolation scaling and arbirary rotation
methods to `Image`.
In the process, several issues with `Image` were found & fixed:

* There was no destructor to free the `canvas` memory. This would
  cause a memory leak unless the client remembered to call `close()`
  A virtual destructor has been added to free this memory.

* There was no explicit copy & move constructor for `Image`
  This means whenever Image was copied or passed by value
  a 'shallow copy' was performed resulting in both instances
  referencing the same shared canvas memory.
  This works, and several examples were depending on it, but
  after the addition of the destructor it caused seg faults since
  the memory was free()'d several times.
  An explicit copy constuctor for `Image` has been added to
  perform a 'deep copy' of the `canvas` memory.
  A C++11 'move' constructor has been added to avoid excessive
  deep copies being made when passing `Image` as a temporary.

* The same logic for copy/move constructors also applies to
  assignment operators (operator=()) so these are added.

* `Image` (and `Color`) are extensively passed by value within
  the library. This now potentially inefficient due to the deep copy.
  (if the move c'tor is not used)
  Since pass-by-reference semantics are required in these cases they
  are now passed by reference.

* Pass-by-(const)-reference changes also required many methods on `Image`
  and `Color` that did not modify their members to be declared `const`
  as well as all `Color` preset constants.

* New c'tors and getters added to `Color` to support RGBA
  access as required by the new scaling method on `Image`
  Also tidied up c'tors to use common helper methods.

* Used new/delete instead of malloc/free for heap memory use  in `Image`
  Just coz it's more conventional in C++
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant