support externally linked (instead of embedded) images #548
Conversation
N-Coder
changed the title
|
Perhaps we should discuss if it is worth adding this complexity to Ipe before we proceed - because, strictly speaking, it does not add new functionality. You can already store your images externally by making a text label with contents
This could be made more comfortable by having the "import image" function create the text object, and we could have a function (or perhaps an ipelet) that externalizes an image that is currently internal. What can be done with |
That is a very good point, as it actually also allows to include PDF pages and all the other arguments of
That sounds like a very good idea!
The only advantage would be that it is much easier to edit programatically, but I do see that throwing around more weirdly-encoded byte buffers is not that nice. So maybe let's take one step back and first think about what we actually want feature- and UX-wise and then think how we want to achieve this through library C++ / ipelet lua code and a maybe updated XML data representation. I'll try to give my 5 cents on this in the linked issue some time later today. |
|
Please change it so there is an additional attribute This is more in line with how |
otfried
force-pushed
the
master
branch
8 times, most recently
from
bc4cb29 to
c8dd95f
Compare
N-Coder
force-pushed
the
linked-images
branch
from
3761de1 to
f0d1c97
Compare
| } | ||
| } | ||
|
|
||
| void Bitmap::changeExternalPathRelativeBase(String new_base) { |
There was a problem hiding this comment.
To keep the code base consistent, I'd prefer camelCase: newBase instead of new_base.
|
|
||
| void Bitmap::changeExternalPathRelativeBase(String new_base) { | ||
| new_base = Platform::realPath(new_base); | ||
| String old_path = Platform::realPath(externalPath()); |
There was a problem hiding this comment.
If we start using <filesystem>, should we just switch to use std::filesystem::absolute instead of Platform::realPath here?
It seems std::filesystem can relieve Ipe of the burden of all the special handling for Windows (where filenames have to be in wchar_t and the separator is a backslash).
| new_base = Platform::realPath(new_base); | ||
| String old_path = Platform::realPath(externalPath()); | ||
| setExternalPath( | ||
| std::filesystem::proximate(old_path.s(), new_base.s()).generic_string()); |
There was a problem hiding this comment.
I guess it should be generic_u8string. In Ipe, all strings are in UTF-8, even if that is not the computer's local encoding, and on Windows it will not work like this.
| reason = EFileOpenError; | ||
| std::FILE * fd = Platform::fopen(fname, "rb"); | ||
| if (!fd) return nullptr; | ||
| Platform::changeDirectory(Platform::parentDirectory(fname)); |
There was a problem hiding this comment.
We cannot simply change the current directory inside Ipe.
There are quite a few people who have ipelets that call bash scripts or that expect the cwd to be in a specific place for some other reason.
We would at least have to set it back after the bitmaps have been loaded.
However, this also seems the wrong place. Just reading the document using Document::load should definitely not load all the bitmaps - imagine this being called from an ipescript that just wants to extract some information.
Loading the bitmaps should only happen when we need to render them.
| if (getcwd(buffer, 1024) != buffer) return String(); | ||
| return String(buffer); | ||
| #endif | ||
| return std::filesystem::current_path().generic_string(); |
There was a problem hiding this comment.
generic_u8string
I'm tempted to say we get rid of Platform::currentDirectory and just call std::filesystem directly in the code, but that's something I can look at later.
otfried
force-pushed
the
master
branch
2 times, most recently
from
d0b335f to
a98414e
Compare
2 participants
This is a first step towards external image support, see also #545.
I think we can copy most of the UX here from the inkscape SVG editor, which already has a very similar functionality:
ToDo: