-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add support for Mac OS #87
base: master
Are you sure you want to change the base?
Conversation
heya, nice, thanks for jumping onto this. as a general comment, the code with these patches applied will probably not run on my machine any more (or on macintosh with intel cpu?). also i want to limit platform specific code to very few places to make our life easier in the future. let me leave a few comments interleaved in the patch. |
src/db/db.c
Outdated
@@ -454,7 +478,7 @@ int dt_db_read(dt_db_t *db, const char *filename) | |||
lno++; | |||
|
|||
// scan filename:rating|labels:number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, macintosh thinks 32-bit are long? then probably better to use %"PRIu64"
here so it will work on all platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just followed compiler warnings, I will see what is going on here, in case ok to switch to PRIu64
src/db/db.h
Outdated
@@ -4,6 +4,12 @@ | |||
#include <string.h> | |||
#include <strings.h> | |||
|
|||
#ifndef __APPLE__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this argument swap issue is terrifying. there has got to be a better way (not involving the ifdef around all compare functions above).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I know, I want just to reach a working/compiling solution and then beutify. 😄
snprintf(cfgfilename, sizeof(cfgfilename), "%s", filename); | ||
snprintf(deffilename, sizeof(deffilename), "default.%"PRItkn, dt_token_str(input_module)); | ||
struct stat statbuf = {0}; | ||
time_t tcfg = 0, tbc1 = 0; | ||
|
||
if(!stat(cfgfilename, &statbuf)) | ||
#ifndef __APPLE__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay we probably want to factor this out in something like time_t t = dt_stat(filename)
defined in core.h
or similar, so the platform specific code is only in one place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I will do something like that to put together all plat-spec code. Thank you for the tip 😉
@@ -314,7 +314,11 @@ void modify_roi_out( | |||
mod->img_param.whitebalance[3] /= mod->img_param.whitebalance[1]; | |||
mod->img_param.whitebalance[1] = 1.0f; | |||
|
|||
#ifndef __APPLE__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really that doesn't exist? then maybe another wrapper in core.h like dt_isnan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes... Too many things are not in Macos... Painful work..
.. about rawspeed. does the vanilla rawspeed git build on M1? i would be surprised if not. anyways it's probably better to use upstream rawspeed for changes. about the blank screen. did you run |
Here I am! If you appreciate it, I'm working to make it compile with CMake, which does more platform-specific controls. I also updated ImGUI dependency (I cherry-picked your modifications) to check for the blank screen problem. Thank you for your feedback! |
With the new imgui this is the error:
Apparently, it cannot set the WindowSize properly... I saw you modified imgui backends for that. |
Updates on error state. I set the font size to 16.0 with a literal to go on. Now blocked at:
I think this is related to how the dynamic libraries of the modules are compiled. |
okay is that error any different to the older imgui at all? not sure i understand how you would put imgui into a gtk3/cairo application. as additional dependency? maybe a saner approach is to use about dynamic libraries. i saw you link there may also be an issue about setting file paths to discover additional data/config files/dsos. i'm highly unlikely to merge cmake build system patches. cmake doesn't do anything for me (it generates makefiles, i have makefiles already), i'm unconvinced it does much for platform independence except mostly a big if around stuff. all variables related to local build environments should go in re:imgui vs glfw functions: some code is c, some is c++ (as minimal as possible, only gui related and interfacing with rawspeed/exiv2). glfw is c, so there are some constraints as to when you can swap things around. |
I'm going to investigate the cause in the following days.
I would like essential ImGui components to control the processing on a single image, then extend for library management like in Ansel. Yes, I would like to get rid of GTK. For sure I will link with
Bad patch due to ramp up on the compilation flow of the project; I will force push the next days.
Thank you for the hint, I will investigate also this.
I will keep it separated. I found it more convenient than hand-written Makefiles for the long-term, but it could be optional for this work. For example, in shared libraries, it automatically handles all the different flags for other OSes.
Here, I need more experience in ImGui development to be helpful. I noticed the patch about |
c7f459d
to
0663632
Compare
Updated with only Makefile modifications. Now the error is:
Same error with updated ImGui (HEAD) and old version (HEAD~2). |
looks like you're lacking |
8322814
to
f93e947
Compare
Now Makefiles are more clean. Next error:
Same error for old and new ImGui+GLFW. Maybe is related to MoltenVK implementation of Vulkan on macOS? |
.. just wanted to ask whether the situation here changed with all the windows/compatibility patches lately? in particular some strange things wrt. |
..maybe to add to the |
Hi @hanatos, I still need to rebase the changes. I spent more time processing photos with Ansel, which now builds ok on M1. I will try to update the PR in few days ;-) |
15f620c
to
9002fbc
Compare
Fast update. I needed to keep a few commits to compiling successfully. Anyway, it is still on a blank screen. I need to investigate more.
|
great, thanks for rebasing!
from experience with the windowws port, i would try to investigate whether
the modules actually load their callbacks (in global.c) and whether any of
them is actually called (for instance the stuff in `i-raw`). another issue
was calling back into symbols contained in the main executable (log access
from i-raw or checking features via `qvk.coopmat_supported` or connecting
nodes and the likes). i suppose that could be tested completely without gui
using the cli too.
…On Tue, Jan 23, 2024 at 9:16 PM Luca Zulberti ***@***.***> wrote:
Fast update. I needed to keep a few commits to compiling successfully.
Anyway, it is still on a blank screen. I need to investigate more.
$ MVK_CONFIG_USE_METAL_ARGUMENT_BUFFERS=1 bin/vkdt -d err -d all -D qvk -D perf ~/Pictures/Temp/ProvaDT/Zulberti.jpg
[pipe] base directory /Users/luca/Git/vkdt/bin
[pipe] home directory /Users/luca/.config/vkdt
[pipe] loaded 76 modules
[gui] glfwGetVersionString() : 3.3.9 Cocoa NSGL EGL OSMesa dynamic
[gui] monitor [0] Built-in Retina Display at 0 0
[gui] vk extension required by GLFW:
[gui] VK_KHR_surface
[gui] VK_EXT_metal_surface
[gui] no joysticks found
[gui] no display profile file display.Built-in Retina Display, using sRGB!
[gui] no display profile file display.Built-in Retina Display, using sRGB!
[db] allocating 1024.0 MB for thumbnails
[mem] images : peak rss 0.00012207 MB vmsize 0.00012207 MB
[mem] buffers: peak rss 0 MB vmsize 0 MB
[mem] staging: peak rss 0.000244141 MB vmsize 0.000244141 MB
[mem] images : peak rss 0.0498505 MB vmsize 0.0498505 MB
[mem] buffers: peak rss 0 MB vmsize 0 MB
[mem] staging: peak rss 0.0997009 MB vmsize 0.0997009 MB
[mem] images : peak rss 144.496 MB vmsize 144.496 MB
[mem] buffers: peak rss 0 MB vmsize 0 MB
[mem] staging: peak rss 91.027 MB vmsize 91.027 MB
[mem] images : peak rss 30.5359 MB vmsize 30.5359 MB
[mem] buffers: peak rss 0 MB vmsize 0 MB
[mem] staging: peak rss 29.3213 MB vmsize 29.3213 MB
[mem] images : peak rss 0.0498505 MB vmsize 0.0498505 MB
[mem] buffers: peak rss 0 MB vmsize 0 MB
[mem] staging: peak rss 0.0997009 MB vmsize 0.0997009 MB
[mem] images : peak rss 144.496 MB vmsize 144.496 MB
[mem] buffers: peak rss 0 MB vmsize 0 MB
[mem] staging: peak rss 91.027 MB vmsize 91.027 MB
[mem] images : peak rss 30.5359 MB vmsize 30.5359 MB
[mem] buffers: peak rss 0 MB vmsize 0 MB
[mem] staging: peak rss 29.3213 MB vmsize 29.3213 MB
—
Reply to this email directly, view it on GitHub
<#87 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMAKKKXCYT4BMS2EB2CMIDYQALDXAVCNFSM6AAAAAA4QZIXNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBWHA2TCNZQHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I also tried to compile this and I also get a blank window. However, the program crashes if I try to open a raw image.
The debug build then crashes at the call of The release build crashes with the following error:
Addendum: When debugging, LLDB also pauses at an exception in I had to apply the following patch to compile with rawspeed. diff --git a/src/pipe/modules/i-raw/flat.mk b/src/pipe/modules/i-raw/flat.mk
index 0a90e242..fbce5052 100644
--- a/src/pipe/modules/i-raw/flat.mk
+++ b/src/pipe/modules/i-raw/flat.mk
@@ -5,12 +5,13 @@ RAWSPEED_L=pipe/modules/i-raw/rawspeed/build
MOD_CFLAGS=-std=c++20 -Wall -I$(RAWSPEED_I)/src/librawspeed/ -I$(RAWSPEED_L)/src/ -I$(RAWSPEED_I)/src/external/ $(VKDT_PUGIXML_CFLAGS) $(VKDT_JPEG_CFLAGS)
MOD_LDFLAGS=-L$(RAWSPEED_L) -lrawspeed -lz $(VKDT_PUGIXML_LDFLAGS) $(VKDT_JPEG_LDFLAGS)
-pipe/modules/i-raw/libi-raw.so: $(RAWSPEED_L)/librawspeed.a
+pipe/modules/i-raw/libi-raw.$(SEXT): $(RAWSPEED_L)/librawspeed.a
-ifeq ($(CXX),clang++)
+CXX_NAME=$(notdir $(CXX))
+ifeq ($(CXX_NAME),clang++)
MOD_LDFLAGS+=-fopenmp=libomp
endif
-ifeq ($(CXX),g++)
+ifeq ($(CXX_NAME),g++)
MOD_LDFLAGS+=-lgomp
endif
@@ -43,7 +44,7 @@ endif # end rawspeed
ifeq ($(VKDT_USE_RAWINPUT),2)
MOD_LDFLAGS=pipe/modules/i-raw/rawloader-c/target/release/librawloader.a
MOD_CFLAGS=-Ipipe/modules/i-raw/rawloader-c
-pipe/modules/i-raw/libi-raw.so: pipe/modules/i-raw/rawloader-c/target/release/librawloader.a
+pipe/modules/i-raw/libi-raw.$(SEXT): pipe/modules/i-raw/rawloader-c/target/release/librawloader.a
pipe/modules/i-raw/rawloader-c/target/release/librawloader.a: pipe/modules/i-raw/rawloader-c/lib.rs pipe/modules/i-raw/rawloader-c/Cargo.toml
cd pipe/modules/i-raw/rawloader-c; cargo update; cargo build --release I hope this helps. |
Another finding: If I remove the
|
turns out on macos the internal framebuffer was running at 1x1 pixel resolution. 84a01f4 fixes this. i can run/use vkdt on an older intel macbook air (using LOD=3). it's not fun but kinda works. i'm not sure this gpu already supports enough metal tier 2 or 3 or whatever to use |
btw, short list of steps to compile on macos, for the curious:
and then it kinda just worked with
|
c3658cf
to
2552bf2
Compare
Hi @hanatos, I saw the update and rebased the PR quickly (poor checks). I also implemented fix from @DorianRudolph, thanks! Error when Rawler is enabled (it seems related to SPIR-V compilation):
|
I still need to use Lunar SDK on my M1 Mac OS Sequoia. Brew-provided MVK seems to have different library names and I did not have the will to investigate and solve dynamic linking problems. |
hmm certainly rawler does not use spir-v, so the error above will resurface if you use rawspeed just the same. this might appear in any module/node of the raw processing pipeline (that is not in use with jpg), so it's hard to tell where exactly things are going wrong. using arrays of descriptors as a scalar descriptor does ring a bell but i don't know where in vkdt's code this might be the case here. |
Dear all, I appreciate your work here and would like to try porting vkDT to Mac OS.
I'm not an experienced Mac OS developer; I want to give it a try to use the software on my MacBook.
My setup:
Ventura 13.5Sequoia 15.0Dependency (to improve the list):
brew
.brew
, installllvm
andlibomp
for custom compilation of therawspeed
library.ToDo:
rawspeed
: add controls for Apple target in 4894513imgui
: cherry-picked modifications from @hanatos fork to upstream in c57af75glfw
: use submodule to compile it with ImGuiUp to now, the commits are just a fast try to make it work.
I know there is a lot of rubbish from many points of view of programmers 😄
Thank you again for this software!