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

Refactor code and create methods to expose functionality that can be reused to implement drag and drop in Windows #69

Closed
wants to merge 1 commit into from

Conversation

martincapello
Copy link
Contributor

@martincapello martincapello commented Apr 11, 2024

  • Separates BitmapInfo implementation from interface, putting code in cpp file and data + methods definitions in header file. It is also moved inside the win namespace. This was needed to be able to reuse the BitmapInfo struct when implementing DragDataProvider::getImage().
  • Moves implementation of write_png_on_stream, write_png, and read_png to cpp file and leaves only definitions on header.
  • Creates create_img and create_dibv5 functions which basically contains code copy & pasted from other methods with some slight changes.

Related to: aseprite/laf#83

Note: I've noted that using vscode for comparing differences is better than using the split view on Github, at least for these changes, because it shows a better match between lines of codes on each side.

…reused to implement drag and drop in Windows
return create_img(bi, output_img);
}

namespace win {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we don't need this namespace here and we could move create_img to the other namespace block. I just did this because the code comparison is actually easier when this code is here (when using vscode to compare changes). If you don't see the point in having this, I will be happy to move it.

@dacap
Copy link
Owner

dacap commented Apr 11, 2024

The clip_win_wic.h header file was created to expose the WICPngEncoder and WICPngDecoder1/WICPngDecoder2 functionality (https://learn.microsoft.com/en-us/windows/win32/wic/-wic-about-windows-imaging-codec).

I think we should create other clip_win.h to expose the other required functions, and add clip_win_wic.cpp to move all the functions that are WIC related there. I can make this refactor, but I'll need what functions you will need from the laf side.

@dacap
Copy link
Owner

dacap commented Apr 11, 2024

I'll check aseprite/laf#83 tomorrow to do this refactor and expose the same set of functions.

@martincapello
Copy link
Contributor Author

martincapello commented Apr 11, 2024

The clip_win_wic.h header file was created to expose the WICPngEncoder and WICPngDecoder1/WICPngDecoder2 functionality (https://learn.microsoft.com/en-us/windows/win32/wic/-wic-about-windows-imaging-codec).

I think we should create other clip_win.h to expose the other required functions, and add clip_win_wic.cpp to move all the functions that are WIC related there. I can make this refactor, but I'll need what functions you will need from the laf side.

Sounds great!

@martincapello
Copy link
Contributor Author

I'll check aseprite/laf#83 tomorrow to do this refactor and expose the same set of functions.

Okay, let me know if you want me to do the refactor.

dacap added a commit that referenced this pull request Apr 12, 2024
These functions might be useful for third-party libraries (like laf,
aseprite/laf#83) to implement a drag-and-drop
functionality.

From: #69

Co-authored-by: Martín Capello <martin@igara.com>
@dacap
Copy link
Owner

dacap commented Apr 12, 2024

I've made the changes in the refactors branch.

The changes in this PR required for laf are introduced in this commit: 4836990 (with some minor changes in the API).

To make aseprite/laf#83 work, you will need these fixes:

diff --git a/os/win/dnd.cpp b/os/win/dnd.cpp
index 001a6a5..b12ce2c 100644
--- a/os/win/dnd.cpp
+++ b/os/win/dnd.cpp
@@ -7,7 +7,9 @@
 #include "os/window.h"
 #include "os/win/dnd.h"
 #include "os/system.h"
-#include "clip/clip_win_wic.h"
+
+#include "clip/clip.h"
+#include "clip/clip_win.h"
 
 #include <shlobj.h>
 
@@ -142,8 +144,8 @@ SurfaceRef DragDataProviderWin::getImage()
   if (m_data->GetData(&fmt, &medium) == S_OK) {
     GLock<BITMAPV5HEADER*> b5(medium.hGlobal);
     clip::win::BitmapInfo bi(b5);
-    if (clip::win::create_img(bi, img)) {
-      bi.fill_spec(spec);
+    if (bi.to_image(img)) {
+      spec = img.spec();
       goto makeSurface;
     }
   }
@@ -152,8 +154,8 @@ SurfaceRef DragDataProviderWin::getImage()
   if (m_data->GetData(&fmt, &medium) == S_OK) {
     GLock<BITMAPINFO*> hbi(medium.hGlobal);
     clip::win::BitmapInfo bi(hbi);
-    if (clip::win::create_img(bi, img)) {
-      bi.fill_spec(spec);
+    if (bi.to_image(img)) {
+      spec = img.spec();
       goto makeSurface;
     }
   }

@dacap dacap closed this Apr 12, 2024
@dacap dacap deleted the refactor-for-win-dnd branch April 22, 2024 18:42
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.

2 participants