wxPDFView - PDFium based PDF viewer controls

[TcT]

Hi,
in my current project I'm in need for a PDF renderer in wxWidgets.
Google recently released sources to Chromes integrated PDF Plugin called
PDFium https://code.google.com/p/pdfium/
It's cross platform based on FoxIT PDF and uses a BSD style license.

I've started a library called wxPDFView with a few components needed to
display PDF content in your application using
the PDFium library.

Currently it includes the following controls:
* wxPDFView Main pdf view
* wxPDFViewBookmarksCtrl tree control displaying bookmarks contained in the
PDF
* wxPDFViewThumbnailsListBox listbox control for displaying thumbnails

This is a first version which already has most of the core functionality.
But there are still a things missing
(Actual thumb display in thumb list box, Search, Text Select, Additional
Zoom Modes, Sample App).
I'll update the repository as my work continues. I hope this might be
useful to somebody beside me :)

Regards,
Tobias

[Vadim Zeitlin]

On Fri, 1 Aug 2014 04:51:16 -0700 (PDT) TcT wrote:

T> I've started a library called wxPDFView with a few components needed to
T> display PDF content in your application using the PDFium library.

Great news! I assume it is the one https://github.com/TcT2k/wxPDFView
here, isn't it? If so, one thing it would be great to have would be some
simple example showing how to use it.

T> Currently it includes the following controls:
T> * wxPDFView Main pdf view
T> * wxPDFViewBookmarksCtrl tree control displaying bookmarks contained in the
T> PDF
T> * wxPDFViewThumbnailsListBox listbox control for displaying thumbnails
T>
T> This is a first version which already has most of the core functionality.
T> But there are still a things missing
T> (Actual thumb display in thumb list box, Search, Text Select, Additional
T> Zoom Modes, Sample App).
T> I'll update the repository as my work continues. I hope this might be
T> useful to somebody beside me :)

It surely will!

Thanks for sharing and good luck!
VZ

[TcT]

Hi,
you are right https://github.com/TcT2k/wxPDFView is the correct URL (forgot
that in the post)

I've added a simple sample project if anybody wants to take a look.
There will be some updates next week to bring this to a relatively stable
state. But if anybody has any input feel free to contact me.

Post by Vadim Zeitlin
T> I've started a library called wxPDFView with a few components needed to
T> display PDF content in your application using the PDFium library.
Great news! I assume it is the one https://github.com/TcT2k/wxPDFView
here, isn't it? If so, one thing it would be great to have would be some
simple example showing how to use it.
T> * wxPDFView Main pdf view
T> * wxPDFViewBookmarksCtrl tree control displaying bookmarks contained in the
T> PDF
T> * wxPDFViewThumbnailsListBox listbox control for displaying thumbnails
T>
T> This is a first version which already has most of the core
functionality.
T> But there are still a things missing
T> (Actual thumb display in thumb list box, Search, Text Select, Additional
T> Zoom Modes, Sample App).
T> I'll update the repository as my work continues. I hope this might be
T> useful to somebody beside me :)
It surely will!
Thanks for sharing and good luck!
VZ
--
TT-Solutions: wxWidgets consultancy and technical support
http://www.tt-solutions.com/

[Vadim Zeitlin]

On Fri, 1 Aug 2014 09:01:25 -0700 (PDT) TcT wrote:

T> Hi,
T> you are right https://github.com/TcT2k/wxPDFView is the correct URL (forgot
T> that in the post)
T>
T> I've added a simple sample project if anybody wants to take a look.

This is really as simple as it gets, nice!

T> There will be some updates next week to bring this to a relatively stable
T> state. But if anybody has any input feel free to contact me.

I had a few questions/comments while looking it. Most of them would
probably be answered by documentation or comments but maybe it would be
still worth pointing them out. Would you prefer to do it here or using
Github comments on the commit itself?

Regards,
VZ

[Tobias Taschner]

Hi,
for now I would prefer questions/comments here over github.

Post by Vadim Zeitlin
T> Hi,
T> you are right https://github.com/TcT2k/wxPDFView is the correct URL (forgot
T> that in the post)
T>
T> I've added a simple sample project if anybody wants to take a look.
This is really as simple as it gets, nice!
T> There will be some updates next week to bring this to a relatively stable
T> state. But if anybody has any input feel free to contact me.
I had a few questions/comments while looking it. Most of them would
probably be answered by documentation or comments but maybe it would be
still worth pointing them out. Would you prefer to do it here or using
Github comments on the commit itself?
Regards,
VZ
--
TT-Solutions: wxWidgets consultancy and technical support
http://www.tt-solutions.com/

[Vadim Zeitlin]

On Fri, 1 Aug 2014 23:29:24 +0200 Tobias Taschner wrote:

TT> for now I would prefer questions/comments here over github.

OK, here are some, in no particular order (i.e. just in order of reading
the code):

- You really should avoid the use of OBJARRAY macros in any new code,
just use a std::vector<> (or wxVector<> if you really plan on using
this code under some platform without std::vector<> implementation),
possibly a vector<unique_ptr<>> if C++11 is allowed or
vector<shared_ptr<>> otherwise (again, wxVector<wxSharedPtr<>> would
work too, it's still a template class and not some macro abomination).

- The meaning of "zoom" parameter is not clear. Is it in percents?

- I think it would be better to have a single ZoomTo(enum ZoomKind) with
ZoomKind_Fit, ZoomKind_FitWidth, ... as this enum elements instead of the
different ZoomFitXXX() functions.

- Why does LoadStream() take the stream by pointer and what does it mean
to take ownership of it?

- I'd strongly consider using "pImpl" design and move all the private
data and methods into wxPDFViewImpl. This would allow to easily preserve
ABI later if wanted (maybe it isn't now but it's much simpler to plan for
it now and not really use it later than to not plan for it and later try
to retrofit it, we have a lot of painful history of wxWidgets to prove it).

- It probably doesn't matter here but using std::list in
wxPDFViewBitmapCache doesn't make much sense to me, IMO it really should
be a vector too.

- If this is _not_ meant to be C++11-only code, then there should be a
space in ">>" closing that std::list type, this is illegal in C++98 as
written.

- I recommend not using wxDataViewTreeCtrl, better use just plain
wxTreeCtrl, wxDataViewTreeCtrl is broken in surprising ways.

- Cast of FPDF_WIDESTRING to wchar_t* looked suspicious and looking at
fpdfview.h it is wrong: FPDF_WIDESTRING is UTF-16LE while wchar_t is
UTF-32 under Unix. You must explicitly use wxMBConvUTF16LE for
converting between it and wxString.

I also have some questions about the threading logic but I'd need to spend
more time reading the code to formulate them clearly, so I'll postpone
writing about them.

Regards,
VZ

[Tobias Taschner]

Post by Vadim Zeitlin
TT> for now I would prefer questions/comments here over github.
OK, here are some, in no particular order (i.e. just in order of reading
- You really should avoid the use of OBJARRAY macros in any new code,
just use a std::vector<> (or wxVector<> if you really plan on using
this code under some platform without std::vector<> implementation),
possibly a vector<unique_ptr<>> if C++11 is allowed or
vector<shared_ptr<>> otherwise (again, wxVector<wxSharedPtr<>> would
work too, it's still a template class and not some macro abomination).

wxList was the first thing I found for a "wxWidgets style" container,
seemed hackish to me but didn't know it was that outdated :)
Changed to std::vector

Post by Vadim Zeitlin
- The meaning of "zoom" parameter is not clear. Is it in percents?

Yes currently is percent

Post by Vadim Zeitlin
- I think it would be better to have a single ZoomTo(enum ZoomKind) with
ZoomKind_Fit, ZoomKind_FitWidth, ... as this enum elements instead of the
different ZoomFitXXX() functions.

Good Idea. Both are currently not implemented I would have to remember the
setting anyway

Post by Vadim Zeitlin
- Why does LoadStream() take the stream by pointer and what does it mean
to take ownership of it?

In my usage scenario the hosting frame creates/destroys the stream. take
ownership simply means that wxPDFView will delete the stream in its dtor

Post by Vadim Zeitlin
- I'd strongly consider using "pImpl" design and move all the private
data and methods into wxPDFViewImpl. This would allow to easily preserve
ABI later if wanted (maybe it isn't now but it's much simpler to plan for
it now and not really use it later than to not plan for it and later try
to retrofit it, we have a lot of painful history of wxWidgets to prove it).

Sounds like a good idea but I'm not yet 100% sure what to put in the impl
and what to keep in the base.

Post by Vadim Zeitlin
- It probably doesn't matter here but using std::list in
wxPDFViewBitmapCache doesn't make much sense to me, IMO it really should
be a vector too.

My thought was that removing entries from cache (when it's filled) should
be fast with a std::list compared to std::vector but I might be wrong there?

Post by Vadim Zeitlin
- If this is _not_ meant to be C++11-only code, then there should be a
space in ">>" closing that std::list type, this is illegal in C++98 as
written.

Will change that

Post by Vadim Zeitlin
- I recommend not using wxDataViewTreeCtrl, better use just plain
wxTreeCtrl, wxDataViewTreeCtrl is broken in surprising ways.
From the documentation the wxDataView backed controls seemed the "modern"

way to go, but I'll reconsider that :)

Post by Vadim Zeitlin
- Cast of FPDF_WIDESTRING to wchar_t* looked suspicious and looking at
fpdfview.h it is wrong: FPDF_WIDESTRING is UTF-16LE while wchar_t is
UTF-32 under Unix. You must explicitly use wxMBConvUTF16LE for
converting between it and wxString.

Just compiled MSW until now as MSW and OSX are the only platforms I use
wxWidgets on, but will obviously change that.
Some wxString character conversions are are a mystery to me until I find
the appropriate method/type cast/helper class.

Post by Vadim Zeitlin
I also have some questions about the threading logic but I'd need to spend
more time reading the code to formulate them clearly, so I'll postpone
writing about them.

I will change some of that logic (as it currently has some flaws). But the
overall idea is to never render the pages in the main thread but to do that
in a background thread an notify UI when requested pages are ready. Also if
you change zoom, bitmaps with the "wrong" resolution are used and scaled
until the correct resolutions are rendered.

Post by Vadim Zeitlin
Regards,
VZ
--
TT-Solutions: wxWidgets consultancy and technical support
http://www.tt-solutions.com/

[Vadim Zeitlin]

On Sat, 2 Aug 2014 12:56:44 +0200 Tobias Taschner wrote:

TT> wxList was the first thing I found for a "wxWidgets style" container,

Please let me know how did you find it, we need to change the
documentation to make it clear that this is not what you should be using.

TT> > - Why does LoadStream() take the stream by pointer and what does it mean
TT> > to take ownership of it?
TT>
TT> In my usage scenario the hosting frame creates/destroys the stream. take
TT> ownership simply means that wxPDFView will delete the stream in its dtor

IMHO it is unusual to pass ownership of the stream objects like this. It's
more common to do it at streambuf level. But if you really want to give the
ownership of the stream itself to PDFView, you should make it more clear in
the function signature, i.e. use auto_ptr<> (C++98) or move semantics
(C++11).

TT> > - I'd strongly consider using "pImpl" design and move all the private
TT> > data and methods into wxPDFViewImpl. This would allow to easily preserve
TT> > ABI later if wanted (maybe it isn't now but it's much simpler to plan for
TT> > it now and not really use it later than to not plan for it and later try
TT> > to retrofit it, we have a lot of painful history of wxWidgets to prove
TT> > it).
TT>
TT> Sounds like a good idea but I'm not yet 100% sure what to put in the impl
TT> and what to keep in the base.

This is simple: only the public methods and overridden virtual methods of
the base class (whether public or not) should remain in the public class.
All the rest, including all the data fields and private methods, should go
into the impl.

TT> > - It probably doesn't matter here but using std::list in
TT> > wxPDFViewBitmapCache doesn't make much sense to me, IMO it really should
TT> > be a vector too.
TT>
TT> My thought was that removing entries from cache (when it's filled) should
TT> be fast with a std::list compared to std::vector but I might be wrong there?

I think in practice std::vector<> would still be faster than std::list<>
for the numbers of elements we're speaking about here (number of pages, so
hundreds, thousands at most), but IMO you don't even need to remove them:
just invalidate the cache entry, i.e. use wxBitmap(), whose IsOk() returns
false, as the bitmap.

TT> > - I recommend not using wxDataViewTreeCtrl, better use just plain
TT> > wxTreeCtrl, wxDataViewTreeCtrl is broken in surprising ways.
TT> >
TT>
TT> From the documentation the wxDataView backed controls seemed the "modern"
TT> way to go, but I'll reconsider that :)

wxDataViewCtrl is fine. So is wxTreeCtrl. But wxDataViewTreeCtrl has quite
a few problems.

TT> > - Cast of FPDF_WIDESTRING to wchar_t* looked suspicious and looking at
TT> > fpdfview.h it is wrong: FPDF_WIDESTRING is UTF-16LE while wchar_t is
TT> > UTF-32 under Unix. You must explicitly use wxMBConvUTF16LE for
TT> > converting between it and wxString.
TT>
TT> Just compiled MSW until now as MSW and OSX are the only platforms I use
TT> wxWidgets on, but will obviously change that.
TT> Some wxString character conversions are are a mystery to me until I find
TT> the appropriate method/type cast/helper class.

I'm trying since years to make this more clear but apparently without
much success :-( IMO it's really simple: the text can be stored either as
wide (wchar_t) string or narrow (char) string. For wchar_t the encoding
used is fixed at compile time and is UTF-16 under Windows and UTF-32
everywhere else, so converting them to/from wxString just works. For char
strings, the encoding may be just about anything (including UTF-16 or
UTF-32) and so you need to specify it explicitly when creating wxString
from them or asking wxString to produce something of this type. If you
don't do this, the current locale encoding is used by default, which is
often, but not always what you want. In any case, it's always better to
be explicit and in this case you know that FPDF_WIDESTRING is UTF-16LE, so
you must always use wxMBConvUTF16LE to convert to/from it.

TT> > I also have some questions about the threading logic but I'd need to spend
TT> > more time reading the code to formulate them clearly, so I'll postpone
TT> > writing about them.
TT>
TT> I will change some of that logic (as it currently has some flaws). But the
TT> overall idea is to never render the pages in the main thread but to do that
TT> in a background thread an notify UI when requested pages are ready. Also if
TT> you change zoom, bitmaps with the "wrong" resolution are used and scaled
TT> until the correct resolutions are rendered.

Yes, this is definitely a good idea, I'm just not sure that the current
code is totally correct. But if you're going to change it anyhow, I'll wait
for the new version before diving into it again.

Good luck!
VZ

[Tobias Taschner]

Post by Vadim Zeitlin
TT> wxList was the first thing I found for a "wxWidgets style" container,
Please let me know how did you find it, we need to change the
documentation to make it clear that this is not what you should be using.

I was looking for wxArray because of classes like wxArrayString thats how I
found it. Not sure if other people would find it that way.

Post by Vadim Zeitlin
TT> > - Why does LoadStream() take the stream by pointer and what does it mean
TT> > to take ownership of it?
TT>
TT> In my usage scenario the hosting frame creates/destroys the stream. take
TT> ownership simply means that wxPDFView will delete the stream in its dtor
IMHO it is unusual to pass ownership of the stream objects like this. It's
more common to do it at streambuf level. But if you really want to give the
ownership of the stream itself to PDFView, you should make it more clear in
the function signature, i.e. use auto_ptr<> (C++98) or move semantics
(C++11).

Changed it to wxSharedPtr

Post by Vadim Zeitlin
TT> > - I'd strongly consider using "pImpl" design and move all the private
TT> > data and methods into wxPDFViewImpl. This would allow to easily preserve
TT> > ABI later if wanted (maybe it isn't now but it's much simpler to plan for
TT> > it now and not really use it later than to not plan for it and later try
TT> > to retrofit it, we have a lot of painful history of wxWidgets to prove
TT> > it).
TT>
TT> Sounds like a good idea but I'm not yet 100% sure what to put in the impl
TT> and what to keep in the base.
This is simple: only the public methods and overridden virtual methods of
the base class (whether public or not) should remain in the public class.
All the rest, including all the data fields and private methods, should go
into the impl.

With your advice it was really simple :)

Post by Vadim Zeitlin
TT> > - It probably doesn't matter here but using std::list in
TT> > wxPDFViewBitmapCache doesn't make much sense to me, IMO it really should
TT> > be a vector too.
TT>
TT> My thought was that removing entries from cache (when it's filled) should
TT> be fast with a std::list compared to std::vector but I might be wrong there?
I think in practice std::vector<> would still be faster than std::list<>
for the numbers of elements we're speaking about here (number of pages, so
just invalidate the cache entry, i.e. use wxBitmap(), whose IsOk() returns
false, as the bitmap.
TT> > - I recommend not using wxDataViewTreeCtrl, better use just plain
TT> > wxTreeCtrl, wxDataViewTreeCtrl is broken in surprising ways.
TT> >
TT>
TT> From the documentation the wxDataView backed controls seemed the "modern"
TT> way to go, but I'll reconsider that :)
wxDataViewCtrl is fine. So is wxTreeCtrl. But wxDataViewTreeCtrl has quite
a few problems.
TT> > - Cast of FPDF_WIDESTRING to wchar_t* looked suspicious and looking at
TT> > fpdfview.h it is wrong: FPDF_WIDESTRING is UTF-16LE while wchar_t is
TT> > UTF-32 under Unix. You must explicitly use wxMBConvUTF16LE for
TT> > converting between it and wxString.
TT>
TT> Just compiled MSW until now as MSW and OSX are the only platforms I use
TT> wxWidgets on, but will obviously change that.
TT> Some wxString character conversions are are a mystery to me until I find
TT> the appropriate method/type cast/helper class.
I'm trying since years to make this more clear but apparently without
much success :-( IMO it's really simple: the text can be stored either as
wide (wchar_t) string or narrow (char) string. For wchar_t the encoding
used is fixed at compile time and is UTF-16 under Windows and UTF-32
everywhere else, so converting them to/from wxString just works. For char
strings, the encoding may be just about anything (including UTF-16 or
UTF-32) and so you need to specify it explicitly when creating wxString
from them or asking wxString to produce something of this type. If you
don't do this, the current locale encoding is used by default, which is
often, but not always what you want. In any case, it's always better to
be explicit and in this case you know that FPDF_WIDESTRING is UTF-16LE, so
you must always use wxMBConvUTF16LE to convert to/from it.

I don't think this is a documentation problem. It's more my problem to
stick with the first way I find to convert something to wxString not
thinking about how it might workout on different platforms/compilers.

Post by Vadim Zeitlin
TT> > I also have some questions about the threading logic but I'd need to spend
TT> > more time reading the code to formulate them clearly, so I'll postpone
TT> > writing about them.
TT>
TT> I will change some of that logic (as it currently has some flaws). But the
TT> overall idea is to never render the pages in the main thread but to do that
TT> in a background thread an notify UI when requested pages are ready. Also if
TT> you change zoom, bitmaps with the "wrong" resolution are used and scaled
TT> until the correct resolutions are rendered.
Yes, this is definitely a good idea, I'm just not sure that the current
code is totally correct. But if you're going to change it anyhow, I'll wait
for the new version before diving into it again.
Good luck!
VZ
--
TT-Solutions: wxWidgets consultancy and technical support
http://www.tt-solutions.com/

[Stefano Mtangoo]

Post by TcT
Hi,
in my current project I'm in need for a PDF renderer in wxWidgets.
Google recently released sources to Chromes integrated PDF Plugin called
PDFium https://code.google.com/p/pdfium/
It's cross platform based on FoxIT PDF and uses a BSD style license.
I've started a library called wxPDFView with a few components needed to
display PDF content in your application using
the PDFium library.
* wxPDFView Main pdf view
* wxPDFViewBookmarksCtrl tree control displaying bookmarks contained in
the PDF
* wxPDFViewThumbnailsListBox listbox control for displaying thumbnails
This is a first version which already has most of the core functionality.
But there are still a things missing
(Actual thumb display in thumb list box, Search, Text Select, Additional
Zoom Modes, Sample App).
I'll update the repository as my work continues. I hope this might be
useful to somebody beside me :)

Wanted just to drop a thank you note!

Post by TcT
Regards,
Tobias