Skip to content
This repository has been archived by the owner on Sep 22, 2020. It is now read-only.

need help from integrating crengine library #77

Closed
houqp opened this issue Mar 28, 2012 · 16 comments
Closed

need help from integrating crengine library #77

houqp opened this issue Mar 28, 2012 · 16 comments

Comments

@houqp
Copy link
Member

houqp commented Mar 28, 2012

I am not familiar with cmake and get stuck in the build system :(

Since we are now using freetype, I think it's better to keep using it as the font engine in CREngine. By default, CREngine enables Fontconfig. To disable it, we need to set the USE_FONTCONFIG macro to 0. But CREngine uses a global crsetup.h header to define macros and since this file is part of upstream, I cannot change its content.

CREngine uses cmake build system, and it seems that arguments from command line like CFLAGS and CC are not recognized by the auto generated makefile.

One possible solution might be adding an outer CMakeLists.txt in kindlepdfviewer's root directory and invoke CREngine's cmake system with custom changes of CFLAGS CC etc. But introducing cmake to kindlepdfviewer is obviously over kill.

Or we can add command in current makefile to temporally modify crsetup.h before building. However, this seems too dirty and it still cannot specify variables like CC.

I have run out of ideas. Any guidance?

PS: in case you want to look at the source, a half-done demo for crengine can be found at my branch.

dpavlin added a commit to dpavlin/kindlepdfviewer that referenced this issue Mar 28, 2012
dpavlin added a commit to dpavlin/kindlepdfviewer that referenced this issue Mar 28, 2012
dpavlin added a commit to dpavlin/kindlepdfviewer that referenced this issue Mar 28, 2012
@dpavlin
Copy link
Member

dpavlin commented Mar 28, 2012

I spend way too much time trying to implement GUI=KINDLEPDFVIEWER target and failing to compile crengine without gui propery. So I decided to hard-code @houqp hack into make fetchthirdparty to provide some help in bootstrap.

My eventual goal is patch to crengine's CMakeLists.txt or new DEVICE_NAME definition which can be submitted upstream or patched on the fly like we had for mupdf.

Android crengine target, on the other hand, includes full Makefile which me might adopt for our needs if we don't want to mess with pushing changes upstream.

@dpavlin dpavlin closed this as completed Mar 28, 2012
@dpavlin dpavlin reopened this Mar 28, 2012
@houqp
Copy link
Member Author

houqp commented Mar 29, 2012

I am for patching to upstream.

@houqp
Copy link
Member Author

houqp commented Mar 29, 2012

We also need to update the jpeg library used by CREngine to release 8d, to keep up with MuPDF.

@houqp
Copy link
Member Author

houqp commented Apr 1, 2012

OK, I just managed to force CREngine use the same jpeglib as mupdf does with dirty hacks.

Firstly, use the headers from mupdf's thirdparty by modifying the CMakeList.txt in crengine:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 94abccb..de41aa1 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -176,8 +176,9 @@ FIND_PACKAGE(JPEG)
 endif (NOT WIN32 AND NOT CR3_JPEG)
 if (NOT JPEG_FOUND)
   message("System LIBJPEG not found, will build local one")
-  ADD_SUBDIRECTORY(thirdparty/libjpeg)
-  SET(JPEG_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/thirdparty/libjpeg)
+  #ADD_SUBDIRECTORY(thirdparty/libjpeg)
+  #SET(JPEG_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/thirdparty/libjpeg)
+  SET(JPEG_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/../mupdf/thirdparty/jpeg)
   SET(JPEG_LIBRARIES jpeg)
 endif (NOT JPEG_FOUND)

Then link mupdf/build/debug/libjpeg.a with kpdfview.

When you open a epub file with jpeg images, you will receive a segfaul. After some debuging, I found that the segfault is caused by line 40-42 in mupdf/thirdparty/jpeg-8d/jdapimin.c. So I comment these lines and segfault is gone.

Now comes the most strange problem. the Decode method of LVJpegImageSource class (in lvimg.cpp) calls a function in jpeglib: jpeg_create_decompress(&cinfo) (in jdapimin.c). This is actually a macro which pass the size of jpeg_decompress_struct to low-level function jpeg_CreateDecompress(). I print the size of jpeg_decompress_struct both before and inside the jpeg_CreateDecompress() call and I got different size!

How can that happen? These two files include the same header file, in which the jpeg_decompress_struct is defined. Why sizeof return two different value?

_EDIT_:
I tried to print the size of another structure defined in the header and I got the same size.

@houqp
Copy link
Member Author

houqp commented Apr 2, 2012

Finally, I decide to use CMake to build all the needed libraries from CREngine (refer to the new commit in my branch). Now no dirty hacks needed for the build environment.

I think the last step towards a successful integration of CREngine into kpview is to fix the segfault pointed out in my previous comment.

@houqp
Copy link
Member Author

houqp commented Apr 3, 2012

Just found out that though my dirty hack get rid of the segfault, it makes all the images in the book disappeared. :(

EDIT: ok, now fixed

@houqp
Copy link
Member Author

houqp commented Apr 4, 2012

Finally, I managed to compile a arm binary with crereader. Now my kindle can read epubs. :) But the dirty hack patch make me very uncomfortable.

@hwhw
Copy link
Member

hwhw commented Apr 7, 2012

I'll also look into this tomorrow. jpeglib's API changed a tad bit.

This is outstanding, awesome work.

dpavlin added a commit to dpavlin/kindlepdfviewer that referenced this issue Apr 7, 2012
dpavlin added a commit to dpavlin/kindlepdfviewer that referenced this issue Apr 7, 2012
It took me few tries to figure out that fonts should point to directory
with *.ttf files as opposed to directory with subdirectories so I
decided to add comment about it
@dpavlin
Copy link
Member

dpavlin commented Apr 7, 2012

I just opened my first epub :-)

@houqp great work, do you think we can merge this in master?

@houqp
Copy link
Member Author

houqp commented Apr 7, 2012

Glad to hear that :)

I think it is not ready to be merged into master yet because I am doing some major rewrite now :P I finally find that it is not a good idea to use percent in side document as absolute position pointer. I am replacing it with XPointer. Hopefully, I can get it done and merge your changes before I go to bed tonight. :)

@dpavlin
Copy link
Member

dpavlin commented Apr 7, 2012

No rush, I was just asking :-)

I have one more problem with make customupdate:

cp -rpL data/ kindlepdfviewer
cp: cannot stat `data/data': No such file or directory

What is especially strange is that adding -v helps:

dpavlin@t61p:/rest/cvs/kindlepdfviewer$ cp -rpvL data/ kindlepdfviewer
`data/epub.css' -> `kindlepdfviewer/data/epub.css'
`data/desktop/cr3.png' -> `kindlepdfviewer/data/desktop/cr3.png'
`data/desktop/cr3.desktop' -> `kindlepdfviewer/data/desktop/cr3.desktop'
# ...a lot more output...

However that also copies a lot of files, including device skins which we don't need, if I'm not mistaken.

@houqp
Copy link
Member Author

houqp commented Apr 7, 2012

That's wired, I don't have that error here. Can you always reproduce it?

However that also copies a lot of files, including device skins which we don't need, if I'm not mistaken.

Yes you are right, I think, for now, we only need those css files. All the rest should be ignored on copying.

@dpavlin
Copy link
Member

dpavlin commented Apr 9, 2012

Should we use Kindle's ttf fonts instead of shipping our own (I must admit, I prefer Kindle ones ;-)

Since we are installed on /mnt/us, we can't just symlink to /usr/java/lib/fonts but mount --bind /usr/java/lib/fonts /mnt/us/kindlepdfviewer/fonts/ works nicely. I looked into luaopen_cre but other than environment variable, I don't have clean way to pass font path in. You might have better solution for this.

@hwhw
Copy link
Member

hwhw commented Apr 9, 2012

I'm just in the course of factoring out the linking of fonts by mupdf. Then, we can distribute those fonts ourselves (at least the Droid fonts) and can build on this and also include these and the platform's fonts. I would rather prefer to not mounting stuff, the cleanup can be a mess since mounts essentially are machine state that doesn't get cleaned up if it isn't explicitly done.

@dpavlin
Copy link
Member

dpavlin commented Apr 9, 2012

No need to mount staff, it should be enough to replace our hard-coded ./fonts with environment lookup.

But, I will wait for you to finish refactoring :-)

@hwhw
Copy link
Member

hwhw commented Apr 9, 2012

Instead, I now implemented bind-mounting... that's because we add our own fonts on top.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants