koreader
koreader copied to clipboard
koreader
epub: svg images not shown
- KOReader version: 2022.06
- Device: Android
Issue
Embedded .svg images in epub are not shown.
Screenshots
KOReader Android screenshot:
E-book viewer (calibre):
Steps to reproduce
Open sicp.epub
It uses <object> that we have no code to handle (dunno how standard this is):
If I modify it to be <img src=...>, we would get:
so, the lines/strokes are there, but not the text. Which is a known limitation of the SVG library we use: https://github.com/koreader/koreader/issues/6757#issuecomment-703932209
So, yes, sadly, our SVG support sucks.
It uses
<object>that we have no code to handle (dunno how standard this is):
That was fairly common back when SVG in the <img> element was still (somewhat) new, and possibly before then as well.
I.e., like this:
<object src="vector.svg" type="image/svg+xml">
<img src="raster-fallback.png">
</object>
I'm surprised to see it combined with <figcaption>.
@poire-z, although http://github.com/memononen/nanosvg/issues/203 requests support for text by the parser of SVG that this project appears to utilize, this reponse to the proposition by whom I believe to be a maintainer of the parser rejects the proposition.
More interesting is the next comment, linking to https://github.com/asmwarrior/SvgPanel which is some continued work based on https://github.com/memononen/nanosvg/pull/94.
Which is something we could do as crengine, the external application, is able to draw text.
Although it feels a bit limited drawing text after all other things, when it may be covered by these other things - but it may be better than not having text, and good enough in real life/books.
I may look at it later this summer.
I'd also watch out for https://github.com/sammycage/lunasvg, which should ultimately gain pretty good text support ;).
should ultimately gain pretty good text support ;).
Nothing in sight :) The maintainer mentionned it had coded up to 60% of it - then lost the code :/ There is some fork that added a bit of text support, but only the parsing and data structures: https://github.com/yurablok/lunasvg Anyway, lunasvg is not actively developped - and its code is libstd++ heavy - looks like another totally alien language to me than the C++ I'm used to with crengine :)
nanosvg also is not actively developped, but it's a bit more graspable. Although, comparing to lunasvg, it is quite limited and not really proper (limited XML parsing, no tree of elements, only a single-level list of shapes, so you don't have inheritance that would be needed for <text><tspan><tspan>.)
But SVG looks so complex, that I guess no little renderer can do it in full (it needs a proper CSS parser and applier, a DOM like tree structure, a text rendering stack with Harfbuzz, and even more complex that what we have in crengine to ensure some SVG options like lengthAdjust and text rotation/transformation, layout text along a textPath that can be an arc, ...), so there must be SVG files where any SVG engine would fail rendering them correctly... A proper SVG rendering engine could be as complex as crengine... :/
I'm tempted to fork nanosvg, pick and cleanup a few of the unmerged PRs, pick and rewrite bits from https://github.com/asmwarrior/SvgPanel to at least get horizontal text.
I managed to get the following with quickly hacking stuff (text in blue is rendering by crengine code over the bitmap):
but there's so many stuff missing (like arrows in the last one), and a few other sample SVG with text fail - that it will always feel "half done and hope you're lucky that your book has simple SVG images"...
But I don't really want to have to maintain/enhance/fix stuff :) So, we could just wait a few more years for a new SVG library to come up that would do it all well. (Does MuPDF support SVG files ? I can't really say looking at their web site.)
MuPDF does, remember we use it for the UI icons. :)
But until we update it support won't exceed nanosvg.
Another nanosvg fork has added more support for SVG stuff, including text and tspan: https://github.com/mtyberg/nanosvg @Frenzie : I see you're one of the 2 person that starred that repo :) Do you remember why, on which occasion, how did you meet it, is it used by some other project... ? No change to the rasterizer, so it must be used by some other software with its own rasterizer. It hasn't been updated for 2 years too...
Another standalone Lua library (that doest not do text either...): https://github.com/Samsung/thorvg/ Another (probably unusable) heavily patched and augmented nanosvg in: https://sourceforge.net/p/cloverefiboot/code/HEAD/tree/rEFIt_UEFI/libeg/
Another nanosvg fork has added more support for SVG stuff, including text and tspan: https://github.com/mtyberg/nanosvg
I tried to work with that one. Synced it with upstream, fixed a few bugs, got things that seemed to work and rendered mostly well, merged a few other PRs like support for clipPath, looked ok on some images - but not ok on others. Once I had it working, I gave it to valgrind, which detected there were many structures not free'd. While investigating the code, I got lost in the so many linked lists. It's quite hell. The original nanosvg is quite limited, so it's quite clean with a limited number of such linked lists, and it's manageable. But once we try to support more stuff like group/defs/use/clippaths, you need more linked lists, some linking the same original structs, and it's a nighmare. After contemplating the mtyberg fork for many hours, I went back to the original simpler nanosv to get the spirit of it - and it's quite nice and simple, clever with its simple linked lists when it only handle plain shapes. But you can'd do much more than handling plain shapes with this linked list implementation... (Also, I have no idea if mtyberg left his fork in a state he was happy with, or if he just gave up :) I assumed the former for a week, but now I'm tending for the latter :) So, I gave up.
Anyway, lunasvg is not actively developped - and its code is libstd++ heavy - looks like another totally alien language to me than the C++ I'm used to with crengine :)
I then tried to add lunasvg to koreader-base and use it from crengine - and most of the things that didn't look right with nanosvg (groups, clipPath) did just look perfect with Lunasvg (except for the absent support for text).
Looks like it is still developped, some bugs are fixed - there's been a large refactoring in early 2021 where it was made simpler and nicer I guess, but it then lost support for text.
I looked at LunaSVG's code, and it is classes based ! and it's heaven trying to understand it. It's clean, nicely structured, except for the std:: stuff everywhere that I'm not familiar with.
But I eventually managed to understand it, and add some preliminary support for the <text> and <tspan> elements.
I had an intuition I could use the help from harfbuzz to ease this text integration, as since v4.0.0, it got a new API https://harfbuzz.github.io/harfbuzz-hb-draw.html which it uses to dump glyph shapes as SVG:
https://github.com/harfbuzz/harfbuzz/blob/4ab7e579cb8f4fd4f5ee2e1c0404e58edf8eb8e6/src/main.cc#L276-L288
I spent the day in despair trying to make the bridge beween lunasvg and crengine and have font->DrawTextString() callback into lunasvg to add a <path d="..."> for each glyph (thought I could use a classic func* for a callback, but I learned I need to use a std::function to get a closure like we are so used with Lua...)
And I finally got my PoC to work ! so sharing my undespair :)
It's quite neat that C++ and LunaSVG (with its proper architecture) allow doing that:
typedef struct {
const char * family;
double size;
bool italic;
int weight;
} external_font_spec_t;
typedef std::function<void(const char * path_d, double advance_x)> tspan_path_callback_t;
typedef void (*external_text_to_paths_func_t)( const char * text, external_font_spec_t * font_spec, tspan_path_callback_t callback );
typedef struct {
external_text_to_paths_func_t text_to_paths_func;
void * dummy;
} external_context_helper_t;
if ( !text.empty() && font_size > 0 ) {
external_font_spec_t font_spec;
std::string font_family = get(PropertyId::Font_Family);
font_spec.family = font_family.c_str();
font_spec.size = Parser::parseLength( find(PropertyId::Font_Size), ForbidNegativeLengths, Length::Zero).value(100.0);
font_spec.italic = false; // XXX
font_spec.weight = 400; // XXX
// This will be called for each glyph
auto add_glyph_callback = tspan_path_callback_t([&](const char * path_d, double advance_x) {
auto path = Parser::parsePath(path_d);
printf("adding shape n=%d advance=%g\n", path.points().size(), advance_x);
auto shape = std::make_unique<LayoutShape>();
shape->path = std::move(path);
// Firefox and Edge don't handle a transform= on a <tspan>, so we can just set ours
shape->transform = Transform::translated(cursor_x, cursor_y);
shape->fillData = context->fillData(this);
shape->strokeData = context->strokeData(this);
shape->markerData = context->markerData(this, shape->path);
shape->visibility = visibility();
shape->clipRule = clip_rule();
shape->opacity = opacity();
shape->masker = context->getMasker(mask());
shape->clipper = context->getClipper(clip_path());
current->addChild(std::move(shape));
cursor_x += advance_x;
});
context->getExternalContextHelper()->text_to_paths_func(text.c_str(), &font_spec, add_glyph_callback);
}
So, I'm confident we can have more proper SVG support in crengine & KOReader.
I guess I'll have to add support for images via some other external crengine helper, for EPUBs that use a SVG wrapper for their cover - which are handled by a hack in crengine currently that I would have to remove to support <svg> embedded in <html>...
@Frenzie @NiLuJe I was about to finish including LunaSVG use by crengine, to discover that the libs have grown unexpectedly :(
koreader + base + crengine current master:
1718464 Sep 1 21:13 liblunasvg.so
21868624 Sep 1 21:19 libcrengine.so
4096 Sep 1 21:19 .
955528 Sep 1 21:19 libkoreader-cre.so
stripped:
141112 Sep 1 21:24 libkoreader-cre.so
4587904 Sep 1 21:24 libcrengine.so
1718464 Sep 1 21:24 liblunasvg.so
koreader+base with the patch below, but crengine & cre.cpp master (so, without any Lunasvg use):
stripped:
1430272 Sep 1 21:31 libkoreader-cre.so <--------- !!!!
3212400 Sep 1 21:31 libcrengine.so
1718464 Sep 1 21:31 liblunasvg.so
readelf on libkoreader-cre.so shows a lot of stdc stuff (including some moneypunct stuff!). Any idea what is happening ?
Here's enough to make this happen (copy & pasted from other section, I have really not much idea what all this does :)
diff --git a/Makefile.defs b/Makefile.defs
index 6031fc1..dcde160 100644
--- a/Makefile.defs
+++ b/Makefile.defs
@@ -796,2 +796,7 @@ LIBWEBP_DIR=$(CURDIR)/$(LIBWEBP_BUILD_DIR)/libwebp-prefix/src/libwebp-build
+LUNASVG_LIB_EXT=$(if $(WIN32),.dll,$(if $(DARWIN),.dylib,.so))
+LUNASVG_LIB=$(OUTPUT_DIR)/libs/liblunasvg$(LUNASVG_LIB_EXT)
+LUNASVG_BUILD_DIR=$(THIRDPARTY_DIR)/lunasvg/build/$(MACHINE)
+LUNASVG_DIR=$(CURDIR)/$(LUNASVG_BUILD_DIR)/lunasvg-prefix/src/lunasvg-build
+
LIBFFI_BUILD_DIR=$(THIRDPARTY_DIR)/libffi/build/$(MACHINE)
@@ -976,3 +981,3 @@ CMAKE_THIRDPARTY_LIBS := $(CMAKE_THIRDPARTY_LIBS),lj-wpaclient
CMAKE_THIRDPARTY_LIBS := $(CMAKE_THIRDPARTY_LIBS),lua-htmlparser,lua-rapidjson,lua-Spore,luasec,luasocket,libffi,glib,lodepng,minizip
-CMAKE_THIRDPARTY_LIBS := $(CMAKE_THIRDPARTY_LIBS),djvulibre,mupdf,freetype2,harfbuzz,giflib,libpng,zlib,libwebp,libwebpdemux
+CMAKE_THIRDPARTY_LIBS := $(CMAKE_THIRDPARTY_LIBS),djvulibre,mupdf,freetype2,harfbuzz,giflib,libpng,zlib,libwebp,lunasvg
CMAKE_THIRDPARTY_LIBS := $(CMAKE_THIRDPARTY_LIBS),tar,sdcv,libiconv,gettext,libjpeg-turbo,popen-noshell,sqlite
diff --git a/Makefile.third b/Makefile.third
index e0548a8..f389726 100644
--- a/Makefile.third
+++ b/Makefile.third
@@ -176,2 +176,16 @@ $(LIBWEBP_LIB) $(LIBWEBP_DIR): $(THIRDPARTY_DIR)/libwebp/*.*
+$(LUNASVG_LIB): $(THIRDPARTY_DIR)/lunasvg/*.*
+ install -d $(LUNASVG_BUILD_DIR)
+ cd $(LUNASVG_BUILD_DIR) && \
+ $(CMAKE) $(CMAKE_FLAGS) \
+ $(CURDIR)/$(THIRDPARTY_DIR)/lunasvg && \
+ $(CMAKE_MAKE_PROGRAM) $(CMAKE_MAKE_PROGRAM_FLAGS)
+ cp -fL $(LUNASVG_DIR)/$(if $(WIN32),,lib)/$(notdir $(LUNASVG_LIB)) $(LUNASVG_LIB)
+ # cp -fL $(THIRDPARTY_DIR)/lunasvg/stb_image_write.h $(LUNASVG_DIR)/include
+ifdef DARWIN
+ install_name_tool -id \
+ libs/$(notdir $(LUNASVG_LIB)) \
+ $(LUNASVG_LIB)
+endif
+
$(DJVULIBRE_LIB): $(JPEG_LIB) $(THIRDPARTY_DIR)/djvulibre/*.*
@@ -194,3 +208,3 @@ endif
$(CRENGINE_LIB): $(ZLIB) $(ZSTD_LIB) $(PNG_LIB) $(FREETYPE_LIB) $(HARFBUZZ_LIB) $(FRIBIDI_LIB) \
- $(LIBUNIBREAK_LIB) $(UTF8PROC_LIB) $(JPEG_LIB) $(NANOSVG_HEADERS) $(LIBWEBP_LIB) \
+ $(LIBUNIBREAK_LIB) $(UTF8PROC_LIB) $(JPEG_LIB) $(LIBWEBP_LIB) $(LUNASVG_LIB) \
$(CRENGINE_SRC_FILES) $(THIRDPARTY_DIR)/kpvcrlib/*.*
@@ -205,2 +219,3 @@ $(CRENGINE_LIB): $(ZLIB) $(ZSTD_LIB) $(PNG_LIB) $(FREETYPE_LIB) $(HARFBUZZ_LIB)
LIBWEBPDEMUX_LIB="$(CURDIR)/$(LIBWEBPDEMUX_LIB)" \
+ LUNASVG_LIB="$(CURDIR)/$(LUNASVG_LIB)" \
LIBUNIBREAK_LIB="$(CURDIR)/$(LIBUNIBREAK_LIB)" \
@@ -214,3 +229,3 @@ $(CRENGINE_LIB): $(ZLIB) $(ZSTD_LIB) $(PNG_LIB) $(FREETYPE_LIB) $(HARFBUZZ_LIB)
-DPNG_INCLUDE_DIR="$(PNG_DIR)/include" \
- -DNANOSVG_INCLUDE_DIR="$(NANOSVG_INCLUDE_DIR)" \
+ -DLUNASVG_INCLUDE_DIR="$(LUNASVG_DIR)/include" \
-DZLIB_INCLUDE_DIR="$(ZLIB_DIR)/include" \
diff --git a/cre.cpp b/cre.cpp
index a11e7ff..f59b43f 100644
--- a/cre.cpp
+++ b/cre.cpp
@@ -3696,2 +3696,3 @@ static int getImageDataFromPosition(lua_State *L) {
unsigned char *cbuf = (unsigned char*) buffer; // cast same void pointer to char pointer
+ /*
// if buffer starts with <?xml or <svg, it's probably SVG
@@ -3712,2 +3713,3 @@ static int getImageDataFromPosition(lua_State *L) {
}
+ */
lua_pushlightuserdata(L, buffer);
diff --git a/thirdparty/kpvcrlib/CMakeLists.txt b/thirdparty/kpvcrlib/CMakeLists.txt
index 29cb6fe..d042175 100644
--- a/thirdparty/kpvcrlib/CMakeLists.txt
+++ b/thirdparty/kpvcrlib/CMakeLists.txt
@@ -32,3 +32,3 @@ assert_var_defined(JPEGLIB_INCLUDE_DIR)
assert_var_defined(LIBWEBP_INCLUDE_DIR)
-assert_var_defined(NANOSVG_INCLUDE_DIR)
+assert_var_defined(LUNASVG_INCLUDE_DIR)
assert_var_defined(ZSTD_INCLUDE_DIR)
@@ -48,3 +48,3 @@ include_directories(
${LIBWEBP_INCLUDE_DIR}
- ${NANOSVG_INCLUDE_DIR}
+ ${LUNASVG_INCLUDE_DIR}
${ZSTD_INCLUDE_DIR}
@@ -82,3 +82,4 @@ add_definitions(
-DUSE_UTF8PROC=1
- -DUSE_NANOSVG=1
+ -DUSE_NANOSVG=0 # better SVG support via LunaSVG
+ -DUSE_LUNASVG=1
-DUSE_LIBWEBP=1
@@ -112,5 +113,6 @@ if(DEFINED ENV{WIN32})
find_library(JPEG_LIB NAMES "jpeg-8" PATHS $ENV{LIBS_DIR})
+ find_library(LUNASVG_LIB NAMES "lunasvg" PATHS $ENV{LIBS_DIR})
find_library(LIBWEBP_LIB NAMES "webp-7" PATHS $ENV{LIBS_DIR})
find_library(LIBWEBPDEMUX_LIB NAMES "webpdemux-2" PATHS $ENV{LIBS_DIR})
- set(THIRDPARTY_LIBS ${ZLIB} ${ZSTD} ${FREETYPE_LIB} ${HARFBUZZ_LIB} ${FRIBIDI_LIB} ${LIBUNIBREAK_LIB} ${UTF8PROC_LIB} ${JPEG_LIB} ${PNG_LIB} ${LIBWEBP_LIB} ${LIBWEBPDEMUX_LIB})
+ set(THIRDPARTY_LIBS ${ZLIB} ${ZSTD} ${FREETYPE_LIB} ${HARFBUZZ_LIB} ${FRIBIDI_LIB} ${LIBUNIBREAK_LIB} ${UTF8PROC_LIB} ${JPEG_LIB} ${PNG_LIB} ${LIBWEBP_LIB} ${LIBWEBPDEMUX_LIB} ${LUNASVG_LIB})
else()
@@ -134,2 +136,4 @@ else()
set_target_properties(LIBWEBPDEMUX_LIB PROPERTIES IMPORTED_LOCATION $ENV{LIBWEBPDEMUX_LIB})
+ add_library(LUNASVG_LIB SHARED IMPORTED)
+ set_target_properties(LUNASVG_LIB PROPERTIES IMPORTED_LOCATION $ENV{LUNASVG_LIB})
add_library(ZLIB SHARED IMPORTED)
@@ -138,3 +142,3 @@ else()
set_target_properties(ZSTD PROPERTIES IMPORTED_LOCATION $ENV{ZSTD_LIB})
- set(THIRDPARTY_LIBS ZLIB ZSTD FREETYPE_LIB HARFBUZZ_LIB FRIBIDI_LIB LIBUNIBREAK_LIB UTF8PROC_LIB JPEG_LIB PNG_LIB LIBWEBP_LIB LIBWEBPDEMUX_LIB)
+ set(THIRDPARTY_LIBS ZLIB ZSTD FREETYPE_LIB HARFBUZZ_LIB FRIBIDI_LIB LIBUNIBREAK_LIB UTF8PROC_LIB JPEG_LIB PNG_LIB LIBWEBP_LIB LIBWEBPDEMUX_LIB LUNASVG_LIB)
endif()
(Even if USE_LUNASVG=1, nothing checks it - there is no #include lunasvg.h anywhere.
Also, there nanosvg is disabled, which may explain the size reduction of libcrengine.so, although that much is strange...)
and
$ cat base/thirdparty/lunasvg/CMakeLists.txt
project(lunasvg)
cmake_minimum_required(VERSION 3.5.1)
set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_CURRENT_LIST_DIR}/../cmake_modules")
include("koreader_thirdparty_common")
include("koreader_thirdparty_git")
enable_language(C CXX)
ep_get_source_dir(SOURCE_DIR)
ep_get_binary_dir(BINARY_DIR)
# CMake hell.
# We expect lib later on in Makefile.third, even on multilib systems...
list(APPEND CMAKE_ARGS "-DCMAKE_INSTALL_PREFIX=${BINARY_DIR}")
list(APPEND CMAKE_ARGS "-DCMAKE_INSTALL_LIBDIR=lib")
list(APPEND CMAKE_ARGS "-DCMAKE_INSTALL_INCLUDEDIR=include")
# list(APPEND CMAKE_ARGS "-DCMAKE_SKIP_BUILD_RPATH:BOOL=True")
# c.f., https://android.googlesource.com/platform/ndk/+/master/build/cmake/android.toolchain.cmake
# and https://github.com/taka-no-me/android-cmake
# In the meantime, I'll be sitting in the corner, crying hysterically.
if(DEFINED ENV{ANDROID})
list(APPEND CMAKE_ARGS "-DCMAKE_SYSTEM_NAME=Linux")
# Magical value that inhibits all of CMake's own NDK handling code. (Or shit goes boom.)
list(APPEND CMAKE_ARGS "-DCMAKE_SYSTEM_VERSION=1")
endif()
list(APPEND CMAKE_ARGS "-DBUILD_SHARED_LIBS=ON")
set(PATCH_CMD1 "${KO_PATCH} ${CMAKE_CURRENT_SOURCE_DIR}/extended.patch")
set(PATCH_CMD2 sh -c "cp -rp -v ${CMAKE_CURRENT_SOURCE_DIR}/xtended ${SOURCE_DIR}/")
ko_write_gitclone_script(
GIT_CLONE_SCRIPT_FILENAME
https://github.com/sammycage/lunasvg.git
637121f89218544ec2f154ae6949ab0ea9b47898
${SOURCE_DIR}
)
include(ExternalProject)
ExternalProject_Add(
lunasvg
DOWNLOAD_COMMAND ${CMAKE_COMMAND} -P ${GIT_CLONE_SCRIPT_FILENAME}
PATCH_COMMAND COMMAND ${PATCH_CMD1} COMMAND ${PATCH_CMD2}
CMAKE_ARGS "${CMAKE_ARGS}"
CMAKE_GENERATOR "Unix Makefiles"
BUILD_COMMAND ${KO_MAKE_RECURSIVE} -j${PARALLEL_JOBS}
INSTALL_COMMAND ${KO_MAKE_RECURSIVE} -j${PARALLEL_JOBS} install
)
(You can remove the 2 PATCH_COMMAND if you wanna try it, they don't do/use anything special not already used in lunasvg...)
Moreover, compiling for my Kobo (with a gcc-8.3 from an older debian, I had bad luck with current debian gcc-10) I also get such big sized libs - and moreover, I get a segfault when I open an epub... (and no gdb on my Kobo). In case it would matter, I get this kind of warnings when compiling lunasvg for arm:
[ 12%] Building CXX object CMakeFiles/lunasvg.dir/source/parser.cpp.o
In file included from gnu_arm-linux-gnueabihf/usr/arm-linux-gnueabihf/include/c++/8/vector:69,
from koreader/base/thirdparty/lunasvg/build/arm-linux-gnueabihf/lunasvg-prefix/src/lunasvg/source/property.h:4,
from koreader/base/thirdparty/lunasvg/build/arm-linux-gnueabihf/lunasvg-prefix/src/lunasvg/source/property.cpp:1:
gnu_arm-linux-gnueabihf/usr/arm-linux-gnueabihf/include/c++/8/bits/vector.tcc: In member function 'void std::vector<_Tp, _Alloc>::_M_realloc_insert(std::vector<_Tp, _Alloc>::iterator, _Args&& ...) [with _Args = {double&, double&}; _Tp = lunasvg::Point; _Alloc = std::allocator<lunasvg::Point>]':
gnu_arm-linux-gnueabihf/usr/arm-linux-gnueabihf/include/c++/8/bits/vector.tcc:413:7: note: parameter passing for argument of type 'std::vector<lunasvg::Point>::iterator' {aka '__gnu_cxx::__normal_iterator<lunasvg::Point*, std::vector<lunasvg::Point> >'} changed in GCC 7.1
vector<_Tp, _Alloc>::
^~~~~~~~~~~~~~~~~~~
gnu_arm-linux-gnueabihf/usr/arm-linux-gnueabihf/include/c++/8/bits/vector.tcc: In member function 'void std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {double&, double&}; _Tp = lunasvg::Point; _Alloc = std::allocator<lunasvg::Point>]':
gnu_arm-linux-gnueabihf/usr/arm-linux-gnueabihf/include/c++/8/bits/vector.tcc:109:4: note: parameter passing for argument of type '__gnu_cxx::__normal_iterator<lunasvg::Point*, std::vector<lunasvg::Point> >' changed in GCC 7.1
_M_realloc_insert(end(), std::forward<_Args>(__args)...);
^~~~~~~~~~~~~~~~~
Any thought (and solution) ? :)
(In spite of the libs sizes, this works wonders on the emulator...)
Yeah, Luna is C++, and we build with -static-libstdc++, so it sorta makes sense.
One possibly saner approach (especially vis-à-vis the ODR) would be to NOT do that, and instead ship the TC's shared libstdc++. The bloat would be limited to the actual STL lib (it's 1.4M over here), and we'd actually be doing C++ properly ;p (e.g., I'm pretty sure what we're currently doing happens to be working by pure blind luck, I fully expect this to horribly crash on a libc++ based TC, for instance).
As for the build error, I can't remember how/when we enforce a default minimum C++ standard version, which sorta coulda explain this, but you'll have better luck just using the koxtoolchain TC ;o).
(I can't easily test debian TCs anyway).
ODR ?
I think there are a few bits of std:: stuff in some of the libs, and I think just the "needed bits" from std get included.
I'd be fine with any std bits getting into liblunasvg.so - and possibly some bits in libcrengine.so. But I don't understand why things get put into libkoreader-cre - when libcrengine is not even including anything liblunasvg. And I made sure to make the cre.cpp change not depending on any std:: stuff - it's supposed to just call libcrengine - which may interact with lunasvg.
So, I think it may be CMake doing too much. No ?
The One Definition Rule ;). (In practical terms: -static-libstdc++ is a terrible idea that *will * horribly crash as soon as two C++ apps built that way interact).
We're passing the actual CRe lib to the CRe Lua module, which is probably how the bloat gets introduced ;).
(I'd need a full build log to confirm; if we're passing -lcrengine, we're doing it right; if we're passing the absolute path to the actual lib: we're doing it wrong).
I'll see if I can throw a PoC about the stdc++ stuff tomorrow (because it is going to blow up in our faces one day regardless), I'll double-check how the Lua module is built in the process ;).
g++ -I/koreader/base/thirdparty/kpvcrlib/crengine/crengine/include/ -L/koreader/base/build/x86_64-linux-gnu-debug/libs -Og -g -pipe -fno-omit-frame-pointer -march=native -fPIC -I/koreader/base/thirdparty/luajit/build/x86_64-linux-gnu-debug/luajit-prefix/src/luajit/src -shared -Wl,-E -Wl,-rpath,'$ORIGIN' \
-DLDOM_USE_OWN_MEM_MAN=1 -DUSE_SRELL_REGEX=1 \
-fvisibility=hidden -static-libstdc++ -o build/x86_64-linux-gnu-debug/libs/libkoreader-cre.so cre.cpp build/x86_64-linux-gnu-debug/libs/libcrengine.so
I guess both lunasvg and libcrengine will need the same std:: bits to be able to talk via std::unique_ptr and std::function :/ But anyway, all this work on the emulator/x86_64.
Actually, just removing -static-libstdc++ from the libkoreader-cre.so build make it small again :) and it still works on the emulator:
$(OUTPUT_DIR)/libs/libkoreader-cre.so: cre.cpp \
$(if $(USE_LUAJIT_LIB),$(LUAJIT_LIB),) \
$(CRENGINE_LIB)
$(CXX) -I$(CRENGINE_SRC_DIR)/crengine/include/ $(DYNLIB_CXXFLAGS) \
-DLDOM_USE_OWN_MEM_MAN=$(if $(WIN32),0,1) -DUSE_SRELL_REGEX=1 \
- $(if $(WIN32),-DQT_GL=1) $(SYMVIS_FLAGS) -static-libstdc++ -o $@ $^
+ $(if $(WIN32),-DQT_GL=1) $(SYMVIS_FLAGS) -o $@ $^
But I guess it won't find that on our Kobo:
libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6
Okay, so we're doing it wrong, yay.
And, yeah, the emulator doesn't need -static-libstdc++ (and, in fact, shouldn't be using it), but we do almost everywhere else (unless we start shipping the TC's STL).
I'll look into it tomorrow ;).
- $(if $(WIN32),-DQT_GL=1) $(SYMVIS_FLAGS) -static-libstdc++ -o $@ $^
+ $(if $(WIN32),-DQT_GL=1) $(SYMVIS_FLAGS) $(if $(USE_LUAJIT_LIB),$(LUAJIT_LIB),) -o $@ cre.cpp -lcrengine
doesn't change much: small and working on the emulator - not small and crashing on my kobo.
(It does weird shit on some binutils version, and it's just not The Right Way(TM) to do things ;)).
Got some sleep, but nothing more :)
I still don't understand, in spite of everything @NiLuJe wrote, why the linker would decide NOW to include moneypunct stuff from stdc into libkoreader-cre.so when nothing even uses lunasvg. LunaSVG is just mentionned in thirdparty/kpvcrlib/CMakeLists.txt - and somehow, this is enough to make that happen. I tried variations aound:
-target_link_libraries(crengine chmlib antiword ${THIRDPARTY_LIBS})
+# target_link_libraries(crengine chmlib antiword ${THIRDPARTY_LIBS})
+# target_link_libraries(crengine PRIVATE -lstdc++ chmlib antiword ${THIRDPARTY_LIBS})
+target_link_libraries(crengine PRIVATE -static-libstdc++ chmlib antiword ${THIRDPARTY_LIBS})
and for the non-CMake build of libkoreader-cre:
- $(CXX) -I$(CRENGINE_SRC_DIR)/crengine/include/ $(DYNLIB_CXXFLAGS) \
+ $(CXX) -I$(CRENGINE_SRC_DIR)/crengine/include/ $(DYNLIB_CXXFLAGS) -nodefaultlibs -fno-exceptions \
-DLDOM_USE_OWN_MEM_MAN=$(if $(WIN32),0,1) -DUSE_SRELL_REGEX=1 \
- $(if $(WIN32),-DQT_GL=1) $(SYMVIS_FLAGS) -static-libstdc++ -o $@ $^
+ $(if $(WIN32),-DQT_GL=1) $(SYMVIS_FLAGS) -static-libstdc++ -lgcc -o $@ $^
(even putting the full resulting command line with the multiple -static-libstdc++ removed) and most of these variations work on the emulator, and get a small libkoreader-cre.so (readelf syaing it only NEEDED libcrengine.so, or libcrengine.so + a few other libm.so, libgcc...) But it still segfault on my Kobo...
I got sidetracked a bit because I decided to reinstall my phone yesterday (EoL'ed by the vendor -> Lineage ;)), but I'll be starting on that now :).
I can't quite answer why which bits of the STL get pulled or not, but the moneypunct stuff very vaguely rings a bell somehow, but I can't quite put my finger on it yet :}.
As for the crash, it's probably because of -static-libstdc++ breaking the ODR.
Thanks ! (Following your progress at https://github.com/NiLuJe/koreader-base/commits/ship_shared_stl) Can you upload here your libstdc++.so.6 , so I have it tomorrow if I wanna try more ? Among the various things I tried this morning, I copied the libstdc++.so.6 from my old debian arm toolchain to my kobo, and got this happening:
error loading module 'libs/libkoreader-cre' from file './libs/libkoreader-cre.so':
/lib/libc.so.6: version `GLIBC_2.18' not found (required by /mnt/onboard/.adds/koreader/libs/libstdc++.so.6)
So, I guess I need a one that will work with the Kobo firmware (including old ones like fw3.19 ?), and I guess that's what your toolchains are all about :) (I hope I can still compile libcrengine/liblunasvg with that old debian arm stuff and have it work with your libstdc++.so.6 ... I really don't want to install that koxtoolchain monstruosity :) last time I thought I would have to make that jump, I realized I wouldn't even have enough disk space :)
Nope, cross-TC (most likely) won't work ;).
FWIW: 252M /home/niluje/x-tools/arm-kobo-linux-gnueabihf
I've got something that builds (and generates smaller binaries, because linker magic).
I've... left the actual CRe build itself alone, because it does something similar (by which I mean passing absolute paths to shared libraries to the final link stage), but it does so via CMake add_library crap, so fuck that ;o).
The leptonica tools also appear to do something similar for libpng, but, again, fuck CMake ;).
That said, you can still try ;).
You can pull the lib from the prebuilt in https://github.com/koreader/koxtoolchain/releases/tag/2021.12 (it'll be @ arm-kobo-linux-gnueabihf/arm-kobo-linux-gnueabihf/sysroot/lib/libstdc++.so.6)
I've... left the actual CRe build itself alone
Might be just a matter of adding -lstdc++ to the last line: ?
target_link_libraries(crengine PRIVATE -lstdc++ chmlib antiword ${THIRDPARTY_LIBS})
FWIW:
252M /home/niluje/x-tools/arm-kobo-linux-gnueabihf
I'll be happy to get that final binary build :) It's building the whole thing that required GBs.
Oh, I didn't even know you have made prebuilt binaries ! I will pick one if need arises !
I've... left the actual CRe build itself alone
Might be just a matter of adding
-lstdc++to the last line: ?target_link_libraries(crengine PRIVATE -lstdc++ chmlib antiword ${THIRDPARTY_LIBS})
Oh, no, I was talking of my crusade against absolute paths, the STL stuff is unnecessary, it'll be picked up automatically ;).
Oh, I didn't even know you have made prebuilt binaries ! I will pick one if need arises !
That's thanks to @yparitcher ;).
Note that, if you can actually still build stuff with your shady TCs, you just need to ship that STL (which is essentially what I'm doing in the branch, the actual lib is pulled from the TC, it's not hard-coded).