pixDisplay() doesn't work under Windows

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Run any program that uses pixDisplay() function.

What is the expected output? What do you see instead?
I expect to be able to view images for debugging.
Instead nothing is displayed.

What version of the product are you using? On what operating system?
leptonlib-1.63.
Microsoft Visual Studio 2008 SP1 with latest updates also applied.
Windows XP Pro SP3.

Please provide any additional information below.
Attached is a changed writefile.c that fixes the problem by assuming that
the user has installed the free imageviewer application IrfanView
(http://www.irfanview.com/) and added it's location to the system PATH. The
README I'm in the process of writing assumes these changes to writefile.c.

Original issue reported on code.google.com by tomp2...@gmail.com on 24 Nov 2009 at 8:23

Attachments:

• writefile.c

[GoogleCodeExporter]

Thanks again!  For 1.64 I've already modified pixDisplay*() to take one of xv, 
xli and 
xzgv, none of which are available on windows.  So adding IrfanView for windows 
is an 
excellent and timely addition to the mix.

There will be a function that you invoke to choose the current default viewer.

Original comment by dan.bloo...@gmail.com on 24 Nov 2009 at 8:31

• Changed state: Started

[GoogleCodeExporter]

I guess it's alright if you can call a function to change the viewer (rather 
than
editing writefile.c). But by default, under Windows, it should use IrfanView.

Otherwise, in order to try any of the prog directory programs that use 
pixDisplay(),
you'd first have to edit the program. Any new users wouldn't realize why 
nothing was
being displayed.

Attached is an extraction of the relevant part of my upcoming README.

Original comment by tomp2...@gmail.com on 24 Nov 2009 at 9:06

Attachments:

• irfanview.html

[GoogleCodeExporter]

It looks fine.  When you've finished the windows readme (including the 
build/link
parts) I'll modify it for the current status of linux display.

We can select the default viewer in writefile.c depending on the compiler flag.

And if you wish, you get your name on the windows readme.  There's a precedent 
for
this, and I prefer to give this credit -- for example, see Brockman's 
documentation file:

    http://leptonica.org/highlevel.html

Original comment by dbloomb...@google.com on 25 Nov 2009 at 4:17

[GoogleCodeExporter]

I've attached the current version of writefile.c, including the modifications to
pixWrite() for adding the extension and to pixDisplay() for using 3+1 display 
programs.

Question: how should we be tagging the windows-only sections?
Should we use #ifdef WIN32 throughout, rather than
#if COMPILER_MSVC?  What is the most simple, consistent way to do this?

Original comment by dan.bloo...@gmail.com on 3 Dec 2009 at 7:12

Attachments:

• writefile.c

[GoogleCodeExporter]

I was wondering the same thing about WIN32 vs COMPILER_MSVC when I realized 
that my
VS2008notes.html and .vcproj file don't define COMPILER_MSVC (or snprintf) when
building leptonlib-1.63\prog programs.

After doing a bit of research, I see from "C/C++ Preprocessor Reference 
Predefined
Macros" (http://msdn.microsoft.com/en-us/library/b0084kay.aspx) that _WIN32 and
_MSC_VER are the only "predefined" macros that meet our needs. WIN32 seems to be
automatically added by the C/C++ New Project wizards, but it can be absent from
manually compiled builds.

I personally like WIN32 (or _WIN32) since they're more well-known, but the case 
might
be made they aren't necessarily specific just to the MSVC compiler? I suppose it
depends on whether any of the other supported compilers also define _WIN32 and 
WIN32
and whether it makes a difference given the way leptonica uses those 
definitions.

_MSC_VER otoh is compiler specific, but it's perhaps too easy to accidentally 
say
MSC_VER instead of _MSC_VER.

Both _WIN32 and _MSC_VER are defined at least back to Visual Studio 2003 (and 
are
also defined in the upcoming VS 2010).

After thinking about it, the safest thing is for leptonica to use COMPILER_MSVC
consistently (and mention in the README that you're using that instead of WIN32 
or
_WIN32). Then you have complete control over its semantics.

If you agree, I need to make a few minor changes to VS2008notes.html and the 
.vcproj
files.

---

While looking at writefile.c I was a bit surprised that

    snprintf(buffer, L_BUF_SIZE, "rm -f /tmp/junk_display.*");

seems to work (at least I didn't notice any error messages about it before).

Turns out I've installed cygwin on my system which has a bunch of unixy type
programs, including rm. I suppose I should add this to VS2008notes.html. I 
added all
the default cygwin stuff which is undoubtedly overkill. I'll need to 
investigate the
minimum packages people need.

It also looks like I need to add 

  /D "snprintf=_snprintf"

to VS2008notes.html and my .vcproj when building leptonlib-1.63\prog programs.

---

Is chooseDisplayProg() ever called by leptonica? I notice that it probably 
won't do
the correct thing under Windows.

Original comment by tomp2...@gmail.com on 3 Dec 2009 at 10:00

[GoogleCodeExporter]

On third thought, I'm a bit uneasy about this change to pixWrite() because it 
will 
certainly break existing programs.

So my current proposal is two-fold:
(1) have a preprocessor variable control whether the names change or not, with
    the initial (default) to no changes.  Anyone who wants extension appending can 
    change this before compiling.
(2) I'll change every output filename in prog/ to append the correct extension.

Attached is the new writefile.c according to (1).

Question: for (2), should I uniformly put all these output files in /tmp?

Original comment by dan.bloo...@gmail.com on 4 Dec 2009 at 12:40

Attachments:

• writefile.c

[GoogleCodeExporter]

In answer to your comment 5 questions:

(1) yes, let's go with COMPILER_MSVC, for the reasons you gave
(2) I prefer snprintf() because it's safer.  As for cygwin, when
    I've used it, it was easiest to include everything.
(3) chooseDisplayProg() is a new function that lets you choose from
    among the unix display programs.  If you have windows, you just
    get i_view by default.

Original comment by dan.bloo...@gmail.com on 4 Dec 2009 at 12:59

[GoogleCodeExporter]

Yeah, I guess if a later routine tries to open filename, it won't be able to 
open
filename.ext. I suppose you could write both--with and without the extension 
(bleh).

(2) Putting all output files in /tmp sounds like a good idea. Currently the prog
directory gets filled up with all sorts of image files after you run a few of 
the
prog programs. IMO the src and prog directories really ought to be "readonly" 
(except
for auto-generated .c files).

You might even consider using your mainName variable as a unique prefix for each
program's output files. A further complication would be to put _reg.c program 
output
in a /tmp/regressionTest directory. Then theoretically you could just do a 
compare of
the entire directory to make sure all the output files are still correct.

Original comment by tomp2...@gmail.com on 4 Dec 2009 at 1:04

[GoogleCodeExporter]

All good suggestions for the regression tests.  The golden files
are TBD.

We've settled on COMPILER_MSVC throughout.

Although automatically adding file extensions is not the
default, this can be enabled by changing WRITE_AS_NAMED to 0.

Fixed for 1.64

Original comment by dan.bloo...@gmail.com on 3 Jan 2010 at 11:30

• Changed state: Fixed