Re: [Plplot-devel] Qt5 support, strange driver output

[António Rodrigues Tomé]

Hi Alan
you must revisit the -bg comand argument.
you are right with your command either with the changed version or with the
original version one gets the opaque blue
but let us put exactly the same directly  in code.
   plscolbga(0,0,255,0.1);
plinit();

and then perform the command
./x30c -dev pngqt -o X30bg00f03_qtChangedplscolbga.png -fam

I attach the result.

P.s.

may it be that the -bg argument is only called after the device constructor
has been valled?


cheers


On Thu, Dec 27, 2018 at 12:22 AM Alan W. Irwin 
wrote:

> On 2018-12-26 22:27- António Rodrigues Tomé wrote:
>
> > HI Alan
> > I'll try to answer your question. But first a little clarification in my
> > system example 30 works exactly the same way with my changes or without
> > them.
> > But really that was not the question.  just try to  put in xo1c example
> > plscolbga(0,0,0,0.);
> >
> > (alpha=0 expected a background transparency) put this beforeplstar(
> 2,
> > 2 );
> >
> > attach the result with my changes and without my changes.
>
> Hi António:
>
> Here is how I applied your commit here and locally modified it on my own
> local topic branch.  (I give git details since I thought you might be
> interested
> in those).
>
> # Start with fresh copy of local master branch so it doesn't interfere with
> # the other PLplot development work I am doing on a different topic branch
> software@raven> git checkout master
> Switched to branch 'master'
> Your branch is up to date with 'origin/master'.
> software@raven> git checkout -b qt5
> software@merlin> git checkout -b qt5
> Switched to a new branch 'qt5'
> # Remind myself how to use "git am"
> software@merlin> git help am
> # Apply your commit
> software@merlin> git am
> <~irwin/António.Rodrigues.Tomé/20181226/0001-correction-in-QtRasterDevice-QtRasterDevice-to-fix-a.patch
>
> Applying: correction in QtRasterDevice::QtRasterDevice to fix a alpha
> problem in the raster qt Drivers
> .git/rebase-apply/patch:19: trailing whitespace.
>  fill(Qt::transparent);
> .git/rebase-apply/patch:30: trailing whitespace.
>
> .git/rebase-apply/patch:31: trailing whitespace.
>
> .git/rebase-apply/patch:34: trailing whitespace.
>
> warning: 4 lines add whitespace errors.
>
> # See what your commit message looks like currently.
>
> commit 11caa19fb0666706794aa955d1a0657d7ec4d54c (HEAD -> qt5)
> Author: António R. Tomé 
> Date:   Mon Dec 24 14:58:00 2018 +
>
>  correction in QtRasterDevice::QtRasterDevice to fix a alpha problem
> in the raster qt Drivers
>
> # Fix trailing whitespace issues
> software@merlin> scripts/remove_trailing_whitespace.sh
> The following list of files have trailing white space
> ./bindings/qt_gui/plqt.cpp
> Remove trailing whitespace from all those files (yes/no)? yes
>
> # Find current status
> software@merlin> git status
> On branch qt5
> Changes not staged for commit:
>(use "git add ..." to update what will be committed)
>(use "git checkout -- ..." to discard changes in working
> directory)
>
>  modified:   bindings/qt_gui/plqt.cpp
>
> no changes added to commit (use "git add" and/or "git commit -a")
>
> # Amend your commit with those whitespace changes
> software@merlin> git add bindings
> software@merlin> git status
> On branch qt5
> Changes to be committed:
>(use "git reset HEAD ..." to unstage)
>
>  modified:   bindings/qt_gui/plqt.cpp
>
> software@merlin> git commit --amend
> [qt5 1ea8ea0a2] correction in QtRasterDevice::QtRasterDevice to fix a
> alpha problem in the raster qt Drivers
>   Author: António R. Tomé 
>   Date: Mon Dec 24 14:58:00 2018 +
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> # Style your commit to get rid of one additional whitespace change you
> made.
> software@merlin> scripts/style_source.sh --apply
> bindings/qt_gui/plqt.cpp
>
> The --apply option is POWERFUL and will replace _all_ files mentioned above
> (if any) with their styled versions.
>
> Continue (yes/no)? yes
> software@merlin> git status
> On branch qt5
> Changes not staged for commit:
>(use "git add ..." to update what will be committed)
>(use "git checkout -- ..." to discard changes in working
> directory)
>
>  modified:   bindings/qt_gui/plqt.cpp
>
> no changes added to commit (use "git add" and/or "git commit -a")
> software@merlin> git add bindings
> software@merlin> git commit --amend
> [qt5 783c85ab5] correction in QtRasterDevice::QtRasterDevice to fix a
> alpha problem in the raster qt D

Re: [Plplot-devel] Qt5 support, strange driver output

[Alan W. Irwin]

On 2018-12-26 22:27- António Rodrigues Tomé wrote:


HI Alan
I'll try to answer your question. But first a little clarification in my
system example 30 works exactly the same way with my changes or without
them.
But really that was not the question.  just try to  put in xo1c example
plscolbga(0,0,0,0.);

(alpha=0 expected a background transparency) put this beforeplstar( 2,
2 );

attach the result with my changes and without my changes.


Hi António:

Here is how I applied your commit here and locally modified it on my own
local topic branch.  (I give git details since I thought you might be interested
in those).

# Start with fresh copy of local master branch so it doesn't interfere with
# the other PLplot development work I am doing on a different topic branch
software@raven> git checkout master
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
software@raven> git checkout -b qt5
software@merlin> git checkout -b qt5
Switched to a new branch 'qt5'
# Remind myself how to use "git am"
software@merlin> git help am
# Apply your commit
software@merlin> git am <~irwin/António.Rodrigues.Tomé/20181226/0001-correction-in-QtRasterDevice-QtRasterDevice-to-fix-a.patch 
Applying: correction in QtRasterDevice::QtRasterDevice to fix a alpha problem in the raster qt Drivers

.git/rebase-apply/patch:19: trailing whitespace.
fill(Qt::transparent); 
.git/rebase-apply/patch:30: trailing whitespace.


.git/rebase-apply/patch:31: trailing whitespace.

.git/rebase-apply/patch:34: trailing whitespace.

warning: 4 lines add whitespace errors.

# See what your commit message looks like currently.

commit 11caa19fb0666706794aa955d1a0657d7ec4d54c (HEAD -> qt5)
Author: António R. Tomé 
Date:   Mon Dec 24 14:58:00 2018 +

correction in QtRasterDevice::QtRasterDevice to fix a alpha problem in the 
raster qt Drivers

# Fix trailing whitespace issues
software@merlin> scripts/remove_trailing_whitespace.sh 
The following list of files have trailing white space

./bindings/qt_gui/plqt.cpp
Remove trailing whitespace from all those files (yes/no)? yes

# Find current status
software@merlin> git status
On branch qt5
Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

modified:   bindings/qt_gui/plqt.cpp

no changes added to commit (use "git add" and/or "git commit -a")

# Amend your commit with those whitespace changes
software@merlin> git add bindings
software@merlin> git status
On branch qt5
Changes to be committed:
  (use "git reset HEAD ..." to unstage)

modified:   bindings/qt_gui/plqt.cpp

software@merlin> git commit --amend
[qt5 1ea8ea0a2] correction in QtRasterDevice::QtRasterDevice to fix a alpha 
problem in the raster qt Drivers
 Author: António R. Tomé 
 Date: Mon Dec 24 14:58:00 2018 +
 1 file changed, 6 insertions(+), 2 deletions(-)

# Style your commit to get rid of one additional whitespace change you made.
software@merlin> scripts/style_source.sh --apply
bindings/qt_gui/plqt.cpp

The --apply option is POWERFUL and will replace _all_ files mentioned above
(if any) with their styled versions.

Continue (yes/no)? yes
software@merlin> git status
On branch qt5
Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

modified:   bindings/qt_gui/plqt.cpp

no changes added to commit (use "git add" and/or "git commit -a")
software@merlin> git add bindings
software@merlin> git commit --amend
[qt5 783c85ab5] correction in QtRasterDevice::QtRasterDevice to fix a alpha 
problem in the raster qt Drivers
 Author: António R. Tomé 
 Date: Mon Dec 24 14:58:00 2018 +
 1 file changed, 4 insertions(+), 1 deletion(-)

# Now see what your modified commit now looks like.
software@merlin> git diff HEAD^ HEAD
diff --git a/bindings/qt_gui/plqt.cpp b/bindings/qt_gui/plqt.cpp
index 13f582d84..f13f6176e 100644
--- a/bindings/qt_gui/plqt.cpp
+++ b/bindings/qt_gui/plqt.cpp
@@ -519,11 +519,13 @@ void QtPLDriver::setSolid()
 #if defined ( PLD_bmpqt ) || defined ( PLD_jpgqt ) || defined ( PLD_pngqt ) || 
defined ( PLD_ppmqt ) || defined ( PLD_tiffqt ) || defined ( PLD_memqt )
 QtRasterDevice::QtRasterDevice( int i_iWidth, int i_iHeight ) :
 QtPLDriver( i_iWidth, i_iHeight ),
-QImage( i_iWidth, i_iHeight, QImage::Format_RGB32 )
+QImage( i_iWidth, i_iHeight, QImage::Format_ARGB32 )
 {
 // Painter initialised in the constructor contrary
 // to buffered drivers, which paint only in doPlot().
 m_painterP = new QPainter( this );
+fill( Qt::transparent );
+
 QBrush b = m_painterP->brush();
 b.setStyle( Qt::SolidPattern );
 m_painterP->setBrush( b );
@@ -561,6 +563,7 @@ void QtRasterDevice::setBa

Re: [Plplot-devel] Qt5 support, strange driver output

[Alan W. Irwin]

On 2018-12-26 13:00-0800 Alan W. Irwin wrote:

[...]

Here are two alternative suggestions:

"Fix background transparency in the raster qt Drivers"

or

"Fix transparency in the raster qt Drivers"

Let me know which of these you prefer.


P.S.

That should have been "raster qt devices" rather than raster qt Drivers"

This change is consistent with our normal terminology where we refer to
the code in, e.g., drivers/qt.cpp that implements the qt device driver, but
we refer to the individual devices that are implemented by that code as the
pngqt device, etc.

Note your two separate commits are completely independent of each
other so I suggest you reply first to my questions concerning

0001-correction-in-QtRasterDevice-QtRasterDevice-to-fix-a.patch

and wait until that commit is pushed by me before answering my further
questions  concerning

0002-correction-in-initQtApp-to-allow-qt-drivers-to-be-ca.patch

Finally, I really appreciate your successful effort to learn
enough about git so you can send us your git commits in
the convenient "git format-patch" form.

Alan
__
Alan W. Irwin

Programming affiliations with the FreeEOS equation-of-state
implementation for stellar interiors (freeeos.sf.net); the Time
Ephemerides project (timeephem.sf.net); PLplot scientific plotting
software package (plplot.sf.net); the libLASi project
(unifont.org/lasi); the Loads of Linux Links project (loll.sf.net);
and the Linux Brochure Project (lbproject.sf.net).
__

Linux-powered Science
__


___
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel

Re: [Plplot-devel] Qt5 support, strange driver output

[Alan W. Irwin]

On 2018-12-25 23:06- António Rodrigues Tomé wrote:


Hi Alain
sorry I haven carefully read up to the end all the README.developers file
here the patch


Hi António:

Thanks for those commits.  My apologies in advance for the number of questions
I have for you in this reply, but I would appreciate you answering all of them
to help improve my overall knowledge concerning the Qt-related components
of PLplot and also to improve the commit messages associated with your
two commits.

One issue I immediately noticed with your commits is both have only a
one-line commit message, and those should be expanded with a following
paragraph with details and two further Tested by: paragraphs.  I can
do the necessary editing of your commit messages here to make those
changes, but in order to do that I will need additional information
from you as detailed below for your two different commits.

I.  0001-correction-in-QtRasterDevice-QtRasterDevice-to-fix-a.patch

Your current summary line,

"correction in QtRasterDevice::QtRasterDevice to fix a alpha problem in the raster 
qt Drivers"

is too repetitive and also too vague.  Here are two alternative suggestions:

"Fix background transparency in the raster qt Drivers"

or

"Fix transparency in the raster qt Drivers"

Let me know which of these you prefer.  (Note I do plan to mention
QtRasterDevice::QtRasterDevice in the explanatory paragraph which is
why I dropped that phrase from the summary line.)

The reason I used the "background" qualifier for the first alternative
is I can find no problem with the non-background transparency in these
devices.  For example, without your fix I attach test_qt.png.1 (the
first page of the example) generated with

examples/c/x30c -dev pngqt -o test_qt.png -fam

where that result clearly shows the effects of transparency for the
various semi-transparent blocks displayed by that example and also
agrees with the first page displayed at
 which was generated with -dev
pngcairo.

However, if your result there (without your fix) on openSUSE is not
the same, then this may be another case of openSUSE exposing bugs in
the PLplot qt device driver better than Debian (i.e., Debian Qt fixups
and workarounds may be more extensive than those from openSUSE).

The reason I am asking about this in detail is one of your changes
involved moving from QImage::Format_RGB32 ==> QImage::Format_ARGB32.

From the description at

 it appears that is
absolutely the right thing to do (unless you decide later to move to
QImage::Format_ARGB32_Premultiplied for efficiency reasons, but that
is obviously a separate issue).  But from that description it seems
our unfixed RGB32 format should not be able to produce the
semitransparent results we see in the attached plot. But maybe Debian
(and openSUSE?) works around that by automatically switching from
RGB32 to ARGB32 if alpha information is provided?

Your response to these questions and comments will help determine
whether I drop "background" from the summary line and greatly
improve the explanatory paragraph I need to write.

Also, can you explain why you had to add

fill(Qt::transparent);

?  Does that mean in the unfixed version there was no background fill
at all for these devices so Qt was falling back to some opaque
background?

I haven't tested this commit yet, but once I do that
I plan to add the following "Tested by" paragraph.

Tested by: Alan W. Irwin  on Linux
(Debian Testing) .

Could you give me those test details for yourself that I
could add to a paragraph started by

Tested by: António Rodrigues Tomé  on Linux
(openSUSE version number?) 
?

II.  0002-correction-in-initQtApp-to-allow-qt-drivers-to-be-ca.patch

Do you agree with shortening your summary line
from

"correction in initQtApp to allow qt drivers to be called from a qt program"

==>

Allow qt devices to be called from a qt program

with initQtApp mentioned in the explanatory paragraph?

That paragraph does not have to be too long, but can you explain to
me why you need to increment appCounter one additional time?
Is the problem that the devices are decrementing that somehow
when your application is finished with them?  If so, wouldn't a
better solution be to specifically increment appCounter in
each of the qt devices?

The reason I am concerned about these appCounter details is I am
pretty sure your current fix will add a memory leak for non-device Qt
applications such as examples/c++/qt_example.cpp.  That example
already has a memory leak due to

PlotWindow   * win = new PlotWindow( Argc, Argv );

with no corresponding xwin delete, but when I attempted to fix that
recently by deleting win that did not work because some other
destructor currently destroys part of it (as far as I can tell from my
limited Qt/C++ knowledge).  Anyhow, I do not want to make the already
bad situation for that example (which makes it sometimes segfault on
exit) worse due to your