Closed GoogleCodeExporter closed 8 years ago
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
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:
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
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:
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
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:
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
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
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
Original issue reported on code.google.com by
tomp2...@gmail.com
on 24 Nov 2009 at 8:23Attachments: