TIP 166: Reference implementation

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
51 messages Options
123
Reply | Threaded
Open this post in threaded view
|

TIP 166: Reference implementation

Simon Bachmann
Hi!

I wrote a reference implementation for TIP 166 ("Reading and Writing the
Photo Image Alpha Channel").

A diff agains Tk 8.6.6 is attached.

My code implements the TIP as-is (at least, that was the intent :-) ).

Yet, there are some points where I think the TIP should be adapted.
Since I'm new here, I thought its' better to ask what you think about it,
before I actually edit the TIP...

1) Compatibility issues with the proposed change to [imageName get]
---------------------------------------------------------------------
The TIP proposes that the subcommand [imageName get] shall return a list of
four values (red, green, blue, alpha) instead of three as it does currently
(red, green, blue).
This potentially breaks existing code.

Personally, I would be happy with the change as proposed in the TIP, but I
suspecht that the guy who has to maintain a large codebase which makes
extensive use of [imageName get] might have a different opinion ;-)

I guess that the Tcl-ish solution is to keep the current behaviour by
default and implement the change with an option:

     % myImage get 2 4
     0 255 255
     % myImage get 2 4 -withalpha
     0 255 255 180

2) Conflicting alpha values in data passed to [imageName put]
--------------------------------------------------------------
In order to specify transparency values when putting data to an image with
[imageName put], the TIP proposes:
a) alpha suffixes (@A, #X and #XX)
b) three additional color formats: #ARGB, #AARRGGBB and a list form with 3
or 4 integers in the range 0..255.

To make an example, all of these color specifications would be vaild
and would evaluate to the same color and transparency values:

     blue#7
     #770000ff
     #00f@0.467
     {0 0 255 119}

What the TIP does not state explicitly, is how to handle the case where an
alpha suffix is used with a format that already contains a transparency
value. eg:

     #700f@0.1

In the reference implementation I chose to raise an error in such a case.
The reason for this choice: one of the new color formats is a Tcl list.
Now,
adding a suffix to a *list* is an idea that doesn't really make sense to
me.
Thus, the suffix shouldn't be allowed for the list format. Then, why
would we
treat the list format specially? It's simpler not to allow an alpha
suffix for
any of the new formats.

3) What about [imageName data]?
-------------------------------
The TIP does not mention the [imageName data] subcommand. Yet, if no other
format is requested, that command returns the image data in the same
form as
the one used for [imageName put].

I think that the [imageName data] subcommand should be changed as well,
so it
can return image data with transparency information.

One reason is consistency. There's a more important one, though: with the
current implementation, the default data format cannot encode transparency
information.

Consider this:

     % image create photo myImage -file $someImgFile.png
     % image create photo myCopy
     % myCopy put [myImage data]

One would expect that 'myCopy' is identical to 'myImage', right?
Thing is, if 'myImage' has (semi)transparent areas, they won't be equal. In
'myCopy' all pixels will be fully opaque - transparency gets lost
because it
cannot be encoded it the default data format.

I propose to "promote" the default list-of-lists-of-pixels format to a
"real"
image format. I'd opt for 'default' as a name for this format.

 From a user's point of view, this wouldn't change much - the clear-text
list
format would stay the default for [imageName data] and the last one to be
tried for [imageName put].

What would change, is that the format could be explicitly requested with
the
'-format' option. And, as other formats, suboptions could be passed in the
value to the '-format' option.

The only suboption, then, would be '-pixelformat type' (for writing). With
'type' being one of:
  - 'rgb'    Use the '#rrggbb' form (current situation, and default for the
             option)
  - 'argb'   Use the '#aarrggbb' form.
  - 'list'   Use the list form with 4 values.

Whit this, the above example could be changed to:

     % image create photo myImage -file $someImgFile.png
     % image create photo myCopy
     % myCopy put [myImage data -format {default -pixelformat argb}]

Now, 'myCopy' will have the same transparency values as 'myImage'.

4) Get access to the "new" color formats from C
------------------------------------------------
IMHO it would be useful if the C API would provide a procedure to parse a
string that specifies a color/alpha value in any of the forms propsed by
this
TIP.

A real life example where this would come in handy: I wrote a C extenstion
that does several image transformations. For some of these transformations,
a background color and background alpha is needed. I wanted to give the
user
the possibility to specifiy these values, which I did with two options, one
for the color and one for transparency.

It would have been simpler to have just one option, which would accept
color/tranparency values in any of the forms proposed by this TIP. For
this,
an API function that parses such a color string into the red, green,
blue and
alpha components would be very useful.

Here's what I have in mind:

Tk_PhotoParseColor(interp, colorObjPtr, defaultAlpha, redPtr, greenPtr,
bluePtr, alphaPtr);

Tcl_Interp *interp (out)    For error messages.
Tcl_Obj *colorObjPtr (in)    The color string to parse.
unsigned char defaultAlpha    Alpha value to use if the color string does
                 not have transparency information.
unsigned char *redPtr (out)    Store red component at this location
unsigned char *greenPtr (out)    Store green component at this location
unsigned char *bluePtr (out)    Store blue component at this location
unsigned char *alphaPtr (out)    Store alpha value at this location

-------------------------------------------------------

Am wonderng if anyone will actually read all of this...

Anyway - Thanks for your feedback!


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core

tip166.diff (66K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Alexandre Ferrieux
On Sun, Feb 26, 2017 at 10:23 AM, Simon Bachmann
<[hidden email]> wrote:

> Hi!
>
> I wrote a reference implementation for TIP 166 ("Reading and Writing the
> Photo Image Alpha Channel").
>
> A diff agains Tk 8.6.6 is attached.
>
> My code implements the TIP as-is (at least, that was the intent :-) ).
>
> Yet, there are some points where I think the TIP should be adapted.
> Since I'm new here, I thought its' better to ask what you think about it,
> before I actually edit the TIP...
>
> 1) Compatibility issues with the proposed change to [imageName get]
> ---------------------------------------------------------------------
> 2) Conflicting alpha values in data passed to [imageName put]
> --------------------------------------------------------------
> 3) What about [imageName data]?
> -------------------------------
> 4) Get access to the "new" color formats from C
> ------------------------------------------------
>
> Am wondering if anyone will actually read all of this...

Hi Simon,

Though I'm not relevantly into Tk these days, let me just give you a
gut feeling:

 - Digging out a 13-year-old TIP and proposing, not only a reference
implementation, but also what look like sane increments, is awesome.

 - "All this" as you say, is still concise and well written, given the
size of the work underneath. So your post is worth reading.

Sorry for this hollow, non-technical answer; I assume people more
directly concerned (especially the TIP author) will now chime in.

In any case, a big thanks for shouldering that load !

-Alex

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Simon Bachmann
Am 26.02.2017 um 11:20 schrieb Alexandre Ferrieux:

> On Sun, Feb 26, 2017 at 10:23 AM, Simon Bachmann
> <[hidden email]> wrote:
>> Hi!
>>
>> I wrote a reference implementation for TIP 166 ("Reading and Writing the
>> Photo Image Alpha Channel").
>>
>> A diff agains Tk 8.6.6 is attached.
>>
>> My code implements the TIP as-is (at least, that was the intent :-) ).
>>
>> Yet, there are some points where I think the TIP should be adapted.
>> Since I'm new here, I thought its' better to ask what you think about it,
>> before I actually edit the TIP...
>>
>> 1) Compatibility issues with the proposed change to [imageName get]
>> ---------------------------------------------------------------------
>> 2) Conflicting alpha values in data passed to [imageName put]
>> --------------------------------------------------------------
>> 3) What about [imageName data]?
>> -------------------------------
>> 4) Get access to the "new" color formats from C
>> ------------------------------------------------
>>
>> Am wondering if anyone will actually read all of this...
> Hi Simon,
>
> Though I'm not relevantly into Tk these days, let me just give you a
> gut feeling:
>
>   - Digging out a 13-year-old TIP and proposing, not only a reference
> implementation, but also what look like sane increments, is awesome.
>
>   - "All this" as you say, is still concise and well written, given the
> size of the work underneath. So your post is worth reading.
>
> Sorry for this hollow, non-technical answer; I assume people more
> directly concerned (especially the TIP author) will now chime in.
>
> In any case, a big thanks for shouldering that load !
>
> -Alex
>
Salut Alex

Merci pour ta réponse.

It might be hollow and non-technical, but it sure is encouraging and
motivating!

Simon


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Rolf Ade
In reply to this post by Simon Bachmann

[Re-posted, because it seems to not have made it thru to the list.
Apologize, if it pops up two times for you.]

Simon Bachmann <[hidden email]>
writes:
> I wrote a reference implementation for TIP 166 ("Reading and Writing the
> Photo Image Alpha Channel").
>
> A diff agains Tk 8.6.6 is attached.

The patch applies cleanly against tk tag core-8-6-6.

It even applies against tk trunk, with only one rejection (obviously not
crucical) in imgPhoto.test.

Build (on linux, 64-bit, 4.7.1) fails because of a few C99-isms, but
they are all only inline var declaration in for loops. After editing
that out, which was quickly done, the build went smooth.

I don't know about the core team position about C99 but this isn't
a real problem in any means and cleaned up in at max minutes.

> From a first glance over I don't see any new substantial problems in
test suite runs (comparing core-8-6-6 and core-8-6-6 with your patch).

I'd propose, to add your implementation into a branch of the tk
repository (which or course also should mean to give you write access).
There we could hammer out any remaining problems and sort out the
questions you raised below.

Good work, Simon!

> My code implements the TIP as-is (at least, that was the intent :-) ).
>
> Yet, there are some points where I think the TIP should be adapted.
> Since I'm new here, I thought its' better to ask what you think about it,
> before I actually edit the TIP...
>
> 1) Compatibility issues with the proposed change to [imageName get]
> ---------------------------------------------------------------------
> [...]
>
> 2) Conflicting alpha values in data passed to [imageName put]
> --------------------------------------------------------------
> [...]
>
> 3) What about [imageName data]?
> -------------------------------
> [...]
>
> 4) Get access to the "new" color formats from C
> ------------------------------------------------
> [...]

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Simon Bachmann
Am 27.02.2017 um 12:44 schrieb Rolf Ade:

> [Re-posted, because it seems to not have made it thru to the list.
> Apologize, if it pops up two times for you.]
>
> Simon Bachmann <[hidden email]>
> writes:
>> I wrote a reference implementation for TIP 166 ("Reading and Writing the
>> Photo Image Alpha Channel").
>>
>> A diff agains Tk 8.6.6 is attached.
> The patch applies cleanly against tk tag core-8-6-6.
>
> It even applies against tk trunk, with only one rejection (obviously not
> crucical) in imgPhoto.test.
>
> Build (on linux, 64-bit, 4.7.1) fails because of a few C99-isms, but
> they are all only inline var declaration in for loops. After editing
> that out, which was quickly done, the build went smooth.
>
> I don't know about the core team position about C99 but this isn't
> a real problem in any means and cleaned up in at max minutes.
I wasn't aware that Tcl/Tk sources should be C89-compliant and not C99.
Will respect that in the future ;-)
>  From a first glance over I don't see any new substantial problems in
> test suite runs (comparing core-8-6-6 and core-8-6-6 with your patch).
>
> I'd propose, to add your implementation into a branch of the tk
> repository (which or course also should mean to give you write access).
> There we could hammer out any remaining problems and sort out the
> questions you raised below.
That woud be great!
> Good work, Simon!
>
Thanks.


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Simon Bachmann
In reply to this post by Simon Bachmann
Update:
The reference implementation for TIP 166 now is on the fossil repo,
branch 'tip-166'.

Currently, it's the same code as in the diff I posted a few days ago
(with a few cosmetic changes).

More substantial changes might come ;-)

Simon


> Hi!
>
> I wrote a reference implementation for TIP 166 ("Reading and Writing the
> Photo Image Alpha Channel").
>
> A diff agains Tk 8.6.6 is attached.
>
> My code implements the TIP as-is (at least, that was the intent :-) ).
>
> Yet, there are some points where I think the TIP should be adapted.
> Since I'm new here, I thought its' better to ask what you think about it,
> before I actually edit the TIP...

<snip!>

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Donal K. Fellows-2
In reply to this post by Alexandre Ferrieux
On 26/02/2017 10:20, Alexandre Ferrieux wrote:
> Sorry for this hollow, non-technical answer; I assume people more
> directly concerned (especially the TIP author) will now chime in.

I wrote it (long ago) and I approve. If any of the ideas in the TIP
don't work when implemented, the TIP should be changed. :-)

Donal.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core

donal_k_fellows.vcf (241 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Simon Bachmann
Am 03.03.2017 um 10:04 schrieb Donal K. Fellows:
> On 26/02/2017 10:20, Alexandre Ferrieux wrote:
>> Sorry for this hollow, non-technical answer; I assume people more
>> directly concerned (especially the TIP author) will now chime in.
>
> I wrote it (long ago) and I approve. If any of the ideas in the TIP
> don't work when implemented, the TIP should be changed. :-)
It's not that the ideas in the TIP don't work, but there are a few
improvements that could be added.

I'll implement those improvements and edit the TIP accordingly.

In the meantime, if anyone wants to play around with (semi)-transparent
photo images, here's a little demo for TIP 166.
The script implements the "color to transparency" feature for photo
images. It needs the changes in the branch tip-166 to work.

I was surprised how easy and quick it was to write. The hardest part was
to figure out the math :-).

If you are going to try this demo, be aware that it's awfully slow.
Don't use it with an image much larger than 100k Pixels, unless you are
veeery patient!

Simon

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core

color2transparent.tcl (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Simon Bachmann
In reply to this post by Simon Bachmann
Am 26.02.2017 um 10:23 schrieb Simon Bachmann:

> I wrote a reference implementation for TIP 166 ("Reading and Writing the
> Photo Image Alpha Channel").
> Yet, there are some points where I think the TIP should be adapted.
>
I just updated TIP 166, as proposed in my previous post.

The reference implementation on the branch tip-166
(http://core.tcl.tk/tk/timeline?n=100&r=tip-166) has been updated
accordingly.

Short summary of the already discussed changes:
--------------------------------------------------------------------

1) The command [imageName get] shall return a list of three values as
before (red, green and blue values). To retrieve the alpha value as
well, an option '-withalpha' has been added: if that option is passed,
the command returns a list with four elements, the alpha value being the
fourth.

2) Specifying alpha values to [imageName put]: the alpha suffixes are
allowed only with the standard Tk color formats (those accepted by
Tk_GetColor()). Suffixes are useless or even ambiguous with any of the
other color formats.

3) Alpha data can be included in the data returned by [imageName data].
To do this, the default list-of-lists-of-pixel-data format has been made
a regular image data handler. This way, suboptions can be passed to the
handler with additional words to the '-format' option.

Discarded:
----------------

4) Access to the new color formats from C.
After thinking this over, I don't think it is a good idea, after all:
     a) This TIP is about making available alpha information at script
level, not about the C API
     b) The additional color formats are specific to photo images. The C
function I presented,
         would make them available to completely unrelated stuff, wich
could lead to confusion.

Additional change:
--------------------------

5) I changed the positon of the new options to [imageName transparency
set], they are now to be passed after the 'newValue' argument.
With more detail, that means:

Until now, we had:

     <imageName> transparency set x y newValue

The previous version of TIP 166 proposed:

     <imageName> transparency set x y ?-boolean | -alpha? newValue

My proposal is:

     <imageName> transparency set x y newValue ?-boolean | -alpha?

While placing the new options before 'newValue' arguably is the more
natural choice (you may read the command as "set transparency of pixel
at x y to boolean/alpha newValue"), the choice clashes with the
conventional order of parameters in Tk commands, i.e. "mandatory
arguments first, then options.

What's next:
------------------

There's a pretty bad performance issue when passing colors in the new
list format to [imageName put]. I'll have to examine that.

Also, some performance comparisions against 8.6.6 won't hurt :-).

The makefiles for Windows need to be tested (changed because of the
addition of a source file).


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Francois Vogel
Simon Bachmann a écrit le 23/03/2017 à 21:15 :
> I just updated TIP 166, as proposed in my previous post.

I have had a look at your proposal, including the implementation.

All this looks very very nice to me.

I have one question though, regarding the -boolean switch you propose
to add to the transparency subcommands 'get' and 'set'. Is this switch
really needed? I mean, what is the difference between absence of this
switch and presence? What changes between:

     set tr [photoImageInstancetransparencyget 5 5]

and

/set /tr [photoImageInstancetransparencyget 5 5 -boolean]

? Wouldn't 'tr' get the same boolean value in both cases?

> Additional change:
> --------------------------
>
> 5) I changed the positon of the new options to [imageName transparency
> set], they are now to be passed after the 'newValue' argument.
> With more detail, that means:
>
> Until now, we had:
>
>       <imageName> transparency set x y newValue
>
> The previous version of TIP 166 proposed:
>
>       <imageName> transparency set x y ?-boolean | -alpha? newValue
>
> My proposal is:
>
>       <imageName> transparency set x y newValue ?-boolean | -alpha?
>
> While placing the new options before 'newValue' arguably is the more
> natural choice (you may read the command as "set transparency of pixel
> at x y to boolean/alpha newValue"), the choice clashes with the
> conventional order of parameters in Tk commands, i.e. "mandatory
> arguments first, then options.

Hmmm... againthe -boolean switch looks superfluous, isn't it?

Also, why not having:

     <imageName> transparency set ?-alpha? x y newValue

This would be backwards compatible since the -alpha switch is
optional. I think it is more in line with the improvements we add
usually (other opinions on this?).


BTW, this remark could apply as well to other parts of your proposal,
e.g. let's rather have:

     <imageName> get ?-withalpha? x y

instead of

     <imageName> get x y ?-withalpha?


Also, about the spelling of the switches, you suggest'-withalpha' for
'get', but '-alpha' for 'transparency get/set'. Why not the same
switch name?Would be easier to remember a single wording I think.

> What's next:
> ------------------
>
> There's a pretty bad performance issue when passing colors in the new
> list format to [imageName put]. I'll have to examine that.
>
> Also, some performance comparisions against 8.6.6 won't hurt :-).

Looking forward to your benchmarks, thanks for caring about performance.

> The makefiles for Windows need to be tested (changed because of the
> addition of a source file).

For me it does not compile on Win, not because the makefile is
incorrect but because there are some warning treated as errors:

tkImgListFormat.c
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(867)
: error C2220: warning treated as error - no 'object' file generated
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(867)
: warning C4013: 'round' undefined; assuming extern returning int
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(897)
: warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
char', possible loss of data
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(898)
: warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
char', possible loss of data
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(899)
: warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
char', possible loss of data

Can you have a look at those?

Compilation on OS X is OK, I have tested it.

Beside all this I see you have worked a lot on the test files, that is
really nice.

Keep up the good work:-) !

Francois


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Francois Vogel
Francois Vogel a écrit le 26/03/2017 à 13:02 :
> For me it does not compile on Win, not because the makefile is
> incorrect but because there are some warning treated as errors:
>
> tkImgListFormat.c
> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(867)
> : error C2220: warning treated as error - no 'object' file generated
> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(867)
> : warning C4013: 'round' undefined; assuming extern returning int

Seems we had this issue in the past:

     http://core.tcl.tk/tk/info/9dc3d444b7bdf7dd

In tkCanvText.c is can also read:

     #define ROUND(d) ((int) floor((d) + 0.5))

I propose to change the offending line to read:

             suffixAlpha = (unsigned int) floor((fracAlpha * 255) + 0.5);

> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(897)
> : warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
> char', possible loss of data
> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(898)
> : warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
> char', possible loss of data
> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(899)
> : warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
> char', possible loss of data

For these three I believe the following new version should be fine:

         *redPtr = (unsigned char) parsedColor.red;
         *greenPtr = (unsigned char) parsedColor.green;
         *bluePtr = (unsigned char) parsedColor.blue;
         *alphaPtr = (unsigned char) suffixAlpha;

> Beside all this I see you have worked a lot on the test files, that is
> really nice.

With the above changes, testing with TESTFLAGS="-file img*.test
-verbose bepsf", I get a crash ("alloc: invalid block [...]") right
after :

++++ imgListFormat-3.4 PASSED

And another one immediately after :

++++ imgPhoto-4.97 PASSED

So my above proposals to fix the warnings may not be correct after all.

Cheers,
Francois


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Brian Griffin-4

On Mar 26, 2017, at 5:19 AM, Francois Vogel <[hidden email]> wrote:

Francois Vogel a écrit le 26/03/2017 à 13:02 :
For me it does not compile on Win, not because the makefile is
incorrect but because there are some warning treated as errors:

tkImgListFormat.c
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(867)
: error C2220: warning treated as error - no 'object' file generated
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(867)
: warning C4013: 'round' undefined; assuming extern returning int

Seems we had this issue in the past:

    http://core.tcl.tk/tk/info/9dc3d444b7bdf7dd

In tkCanvText.c is can also read:

    #define ROUND(d) ((int) floor((d) + 0.5))

I propose to change the offending line to read:

            suffixAlpha = (unsigned int) floor((fracAlpha * 255) + 0.5);

C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(897)
: warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
char', possible loss of data
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(898)
: warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
char', possible loss of data
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(899)
: warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
char', possible loss of data

For these three I believe the following new version should be fine:

        *redPtr = (unsigned char) parsedColor.red;
        *greenPtr = (unsigned char) parsedColor.green;
        *bluePtr = (unsigned char) parsedColor.blue;
        *alphaPtr = (unsigned char) suffixAlpha;

Wait what?  Isn't this truly a loss of data?  Important data?  I would think conversion of 16-bit color value to 8-bit color must be red>>8 (or red/256).

-Brian

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Simon Bachmann
In reply to this post by Francois Vogel
Am 26.03.2017 um 13:02 schrieb Francois Vogel:

> Simon Bachmann a écrit le 23/03/2017 à 21:15 :
>> I just updated TIP 166, as proposed in my previous post.
> I have had a look at your proposal, including the implementation.
>
> All this looks very very nice to me.
>
> I have one question though, regarding the -boolean switch you propose
> to add to the transparency subcommands 'get' and 'set'. Is this switch
> really needed? I mean, what is the difference between absence of this
> switch and presence? What changes between:
>
>       set tr [photoImageInstancetransparencyget 5 5]
>
> and
>
> /set /tr [photoImageInstancetransparencyget 5 5 -boolean]
>
> ? Wouldn't 'tr' get the same boolean value in both cases?

That's correct, the -boolean switch is redundant. Since I didn't write
the TIP, I can only guess the reasoning behind this. I think it's for
"symmetric" reasons: if we have two variants, both should get their
switch - even if one of them is the default when no switch is passed.

One use case where the -boolean option could be useful, is something
like this:

     myImg transparency get $x $y $type

where $type would be either '-boolean' or '-alpha'.

Yet, I never really felt the need for this -boolean option :-). If I
understand this correctly, this "boolean transparency" is a relic from
days long gone, when photo images didn't have an alpha channel. So I
wouln't mind removing it.

>
>> Additional change:
>> --------------------------
>>
>> 5) I changed the positon of the new options to [imageName transparency
>> set], they are now to be passed after the 'newValue' argument.
>> With more detail, that means:
>>
>> Until now, we had:
>>
>>        <imageName> transparency set x y newValue
>>
>> The previous version of TIP 166 proposed:
>>
>>        <imageName> transparency set x y ?-boolean | -alpha? newValue
>>
>> My proposal is:
>>
>>        <imageName> transparency set x y newValue ?-boolean | -alpha?
>>
>> While placing the new options before 'newValue' arguably is the more
>> natural choice (you may read the command as "set transparency of pixel
>> at x y to boolean/alpha newValue"), the choice clashes with the
>> conventional order of parameters in Tk commands, i.e. "mandatory
>> arguments first, then options.
> Hmmm... againthe -boolean switch looks superfluous, isn't it?
>
> Also, why not having:
>
>       <imageName> transparency set ?-alpha? x y newValue
>
> This would be backwards compatible since the -alpha switch is
> optional. I think it is more in line with the improvements we add
> usually (other opinions on this?).
>
>
> BTW, this remark could apply as well to other parts of your proposal,
> e.g. let's rather have:
>
>       <imageName> get ?-withalpha? x y
>
> instead of
>
>       <imageName> get x y ?-withalpha?

I disagree on this one. Two reasons:

1) While images clearly are not widgets, they do resemble widgets in
functionality (creating an instance creates a command, 'configure' and
'cget' commands, etc.).
Now, AFAIK, for widgets the order always is:

     pathName command arg ... ?option value ...?

2) For all the existing <photoImageInstance> commands, options and
switches always come after the fixed args. Having some of them where the
switches are to be passed in between would be inconsistent and probably
quite confusing.
>
>
> Also, about the spelling of the switches, you suggest'-withalpha' for
> 'get', but '-alpha' for 'transparency get/set'. Why not the same
> switch name?Would be easier to remember a single wording I think.

I did think about that too.

But consider this:

     myImg get $x $y -alpha

Without any previous knowledge, you'd probably expect that the above
command returns the alpha value of pixel at $x,$y.
What it actually does, is to return a list of four values (rgba).

Thing is, the switches are similar, yet they don't have the same effect
in the two cases: for 'transparency get/set' it says to use an alpha
value *instead of* a boolean, for 'get' it requests the alpha value *in
addidtion* to the color values.

I think the actual question is wheter it's better to use the same switch
for two similar yet different operations, or to use similar switches for
similar operations. I opted for the latter, but the first one has its
advantages too.

>
>> What's next:
>> ------------------
>>
>> There's a pretty bad performance issue when passing colors in the new
>> list format to [imageName put]. I'll have to examine that.
>>
>> Also, some performance comparisions against 8.6.6 won't hurt :-).
> Looking forward to your benchmarks, thanks for caring about performance.
>
>> The makefiles for Windows need to be tested (changed because of the
>> addition of a source file).
> For me it does not compile on Win, not because the makefile is
> incorrect but because there are some warning treated as errors:
>
> tkImgListFormat.c
> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(867)
> : error C2220: warning treated as error - no 'object' file generated
> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(867)
> : warning C4013: 'round' undefined; assuming extern returning int
> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(897)
> : warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
> char', possible loss of data
> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(898)
> : warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
> char', possible loss of data
> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(899)
> : warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
> char', possible loss of data
>
> Can you have a look at those?
>
> Compilation on OS X is OK, I have tested it.

Thanks for testing that, I don't have access to OS X.

>
> Beside all this I see you have worked a lot on the test files, that is
> really nice.
>
> Keep up the good work:-) !

Thanks for the feedback!

>
> Francois
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Tcl-Core mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/tcl-core
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Simon Bachmann
In reply to this post by Brian Griffin-4
Am 26.03.2017 um 18:14 schrieb Brian Griffin:

On Mar 26, 2017, at 5:19 AM, Francois Vogel <[hidden email]> wrote:

Francois Vogel a écrit le 26/03/2017 à 13:02 :
For me it does not compile on Win, not because the makefile is
incorrect but because there are some warning treated as errors:

tkImgListFormat.c
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(867)
: error C2220: warning treated as error - no 'object' file generated
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(867)
: warning C4013: 'round' undefined; assuming extern returning int

Seems we had this issue in the past:

    http://core.tcl.tk/tk/info/9dc3d444b7bdf7dd

In tkCanvText.c is can also read:

    #define ROUND(d) ((int) floor((d) + 0.5))

I propose to change the offending line to read:

            suffixAlpha = (unsigned int) floor((fracAlpha * 255) + 0.5);

C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(897)
: warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
char', possible loss of data
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(898)
: warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
char', possible loss of data
C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(899)
: warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
char', possible loss of data

For these three I believe the following new version should be fine:

        *redPtr = (unsigned char) parsedColor.red;
        *greenPtr = (unsigned char) parsedColor.green;
        *bluePtr = (unsigned char) parsedColor.blue;
        *alphaPtr = (unsigned char) suffixAlpha;

Wait what?  Isn't this truly a loss of data?  Important data?  I would think conversion of 16-bit color value to 8-bit color must be red>>8 (or red/256).
Nope, no loss of data. The values do get bitshifted earlier in the code:

    parsedColor.red >>= 8;
    parsedColor.green >>= 8;
    parsedColor.blue >>= 8;

I do realise that this isn't exactly good coding style. Will fix that.


-Brian


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot


_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Alexandre Ferrieux
In reply to this post by Simon Bachmann
On Sun, Mar 26, 2017 at 8:49 PM, Simon Bachmann
<[hidden email]> wrote:
> Thing is, the switches are similar, yet they don't have the same effect
> in the two cases: for 'transparency get/set' it says to use an alpha
> value *instead of* a boolean, for 'get' it requests the alpha value *in
> addidtion* to the color values.
>
> I think the actual question is wheter it's better to use the same switch
> for two similar yet different operations, or to use similar switches for
> similar operations. I opted for the latter, but the first one has its
> advantages too.

FWIW, I quite like the precision brought by distinguishing the two.

-Alex

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Simon Bachmann
In reply to this post by Francois Vogel
Am 26.03.2017 um 14:19 schrieb Francois Vogel:

> Francois Vogel a écrit le 26/03/2017 à 13:02 :
>> For me it does not compile on Win, not because the makefile is
>> incorrect but because there are some warning treated as errors:
>>
>> tkImgListFormat.c
>> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(867)
>> : error C2220: warning treated as error - no 'object' file generated
>> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(867)
>> : warning C4013: 'round' undefined; assuming extern returning int
> Seems we had this issue in the past:
>
>       http://core.tcl.tk/tk/info/9dc3d444b7bdf7dd
>
> In tkCanvText.c is can also read:
>
>       #define ROUND(d) ((int) floor((d) + 0.5))
>
> I propose to change the offending line to read:
>
>               suffixAlpha = (unsigned int) floor((fracAlpha * 255) + 0.5);

Thanks for the hint. Wasn't aware that round() is C99.

>
>> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(897)
>> : warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
>> char', possible loss of data
>> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(898)
>> : warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
>> char', possible loss of data
>> C:\Users\francois\Documents\Development\tcltk-fossil\tk\win\..\generic\tkImgListFormat.c(899)
>> : warning C4244: '=' : conversion from 'unsigned short' to 'unsigned
>> char', possible loss of data
> For these three I believe the following new version should be fine:
>
>           *redPtr = (unsigned char) parsedColor.red;
>           *greenPtr = (unsigned char) parsedColor.green;
>           *bluePtr = (unsigned char) parsedColor.blue;
>           *alphaPtr = (unsigned char) suffixAlpha;
>
>> Beside all this I see you have worked a lot on the test files, that is
>> really nice.
> With the above changes, testing with TESTFLAGS="-file img*.test
> -verbose bepsf", I get a crash ("alloc: invalid block [...]") right
> after :
>
> ++++ imgListFormat-3.4 PASSED
>
> And another one immediately after :
>
> ++++ imgPhoto-4.97 PASSED
>
> So my above proposals to fix the warnings may not be correct after all.
Hmm... Your proposals look fine, I don't think they are the cause of
this. Looks more like a bug somewhere in my code.

>
> Cheers,
> Francois
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Tcl-Core mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/tcl-core
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Francois Vogel
In reply to this post by Simon Bachmann
Simon Bachmann a écrit le 26/03/2017 à 20:49

>> e.g. let's rather have:
>>
>>        <imageName> get ?-withalpha? x y
>>
>> instead of
>>
>>        <imageName> get x y ?-withalpha?
> I disagree on this one. Two reasons:
>
> 1) While images clearly are not widgets, they do resemble widgets in
> functionality (creating an instance creates a command, 'configure' and
> 'cget' commands, etc.).
> Now, AFAIK, for widgets the order always is:
>
>       pathName command arg ... ?option value ...?

Following quick skimming of the man pages for widgets, that's
apparently true. Nearly.

There are counterexamples to this, where mandatory args come after
optional ones, e.g.:

     .text dump ?switches? index1 ?index2?
     .text search ?switches? pattern index ?stopIndex?
     .text yview ?-pickplace? index

But I won't make a strong case of this.

F.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Francois Vogel
In reply to this post by Simon Bachmann
Simon Bachmann a écrit le 26/03/2017 à 20:49
>> Compilation on OS X is OK, I have tested it.
> Thanks for testing that, I don't have access to OS X.

I take this opportunity to thank Brad Lanam again for providing access
to his Mac to me for Tcl/Tk testing from the other side of the world,
that's really great!

On OS X, running the test suite for the img*.test files triggers only
one failure:

==== imgListFormat-6.19 ParseColor: invalid color: not a hex digit FAILED
==== Contents of test case:

     photo1 put {#ABCDEF@.99 #ABCDEG@.99}

---- Test completed normally; Return code was: 0
---- Return code should have been one of: 1
==== imgListFormat-6.19 FAILED

This is repeatable each time on OS X Sierra 10.12.3

Cheers,
Francois


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Francois Vogel
In reply to this post by Simon Bachmann
Simon Bachmann a écrit le 26/03/2017 à 21:08 :

>> With the above changes, testing with TESTFLAGS="-file img*.test
>> -verbose bepsf", I get a crash ("alloc: invalid block [...]") right
>> after :
>>
>> ++++ imgListFormat-3.4 PASSED
>>
>> And another one immediately after :
>>
>> ++++ imgPhoto-4.97 PASSED
>>
>> So my above proposals to fix the warnings may not be correct after all.
> Hmm... Your proposals look fine, I don't think they are the cause of
> this. Looks more like a bug somewhere in my code.

The problem with imgListFormat-3.5 can be seen on Windows by just:

% package require Tk
8.7a0
% image create photo photo1
photo1
% photo1 put {{blue green}
     {yellow magenta}
     {#000000 #FFFFFFFF}}
alloc: invalid block: 02CE5848: ef ef 0


The problem with imagePhoto-4.98 is the same, it can be triggered by:

% package require Tk
8.7a0
% image create photo photo1
photo1
% photo1 put {{"alice blue" "blanched almond"}
         {"deep sky blue" "ghost white"}
         {#AABBCC #AABBCCDD}} -to 5 6
alloc: invalid block: 026C57C8: ef ef 0


In both cases the call stack is:

 >    tcl87tg.dll!tclWinDebugPanic(const char * format=0x1020ce18,
...)  Line 832    C
      tcl87tg.dll!Tcl_PanicVA(const char * format=0x1020ce18, char *
argList=0x002bf870)  Line 104    C
      tcl87tg.dll!Tcl_Panic(const char * format=0x1020ce18, ...) Line
161    C
      tcl87tg.dll!Ptr2Block(char * ptr=0x042c7718)  Line 843    C
      tcl87tg.dll!TclpFree(char * ptr=0x042c7718)  Line 399 + 0x9
bytes    C
      tcl87tg.dll!Tcl_Free(char * ptr=0x042c7718)  Line 1221 + 0x9
bytes    C
      tk87tg.dll!StringReadDef(Tcl_Interp * interp=0x00695bd0, Tcl_Obj
* data=0x042cbed0, Tcl_Obj * formatString=0x00000000, void *
imageHandle=0x0428e368, int destX=5, int destY=6, int width=2, int
height=3, int srcX=0, int srcY=0)  Line 573 + 0x14 bytes    C
      tk87tg.dll!ImgPhotoCmd(void * clientData=0x0428e368, Tcl_Interp
* interp=0x00695bd0, int objc=6, Tcl_Obj * const * objv=0x0069cf34)  
Line 901 + 0x3d bytes    C
      tcl87tg.dll!Dispatch(void * * data=0x042ccce4, Tcl_Interp *
interp=0x00695bd0, int result=0)  Line 4376 + 0x15 bytes    C
      tcl87tg.dll!TclNRRunCallbacks(Tcl_Interp * interp=0x00695bd0,
int result=0, NRE_callback * rootPtr=0x00000000)  Line 4409 + 0x14
bytes    C
      tcl87tg.dll!TclEvalObjEx(Tcl_Interp * interp=0x00695bd0, Tcl_Obj
* objPtr=0x00748618, int flags=131072, const CmdFrame *
invoker=0x00000000, int word=0)  Line 5976 + 0x11 bytes    C
      tcl87tg.dll!Tcl_EvalObjEx(Tcl_Interp * interp=0x00695bd0,
Tcl_Obj * objPtr=0x00748618, int flags=131072)  Line 5957 + 0x15
bytes    C
      tcl87tg.dll!Tcl_RecordAndEvalObj(Tcl_Interp * interp=0x00695bd0,
Tcl_Obj * cmdPtr=0x00748618, int flags=131072) Line 189 + 0x17 bytes    C
      tcl87tg.dll!StdinProc(void * clientData=0x002bfecc, int mask=2)  
Line 811 + 0x12 bytes    C
      tcl87tg.dll!Tcl_NotifyChannel(Tcl_Channel_ * channel=0x006cb508,
int mask=2)  Line 8319 + 0x1b bytes    C
      tcl87tg.dll!ConsoleEventProc(Tcl_Event * evPtr=0x03045c30, int
flags=-3)  Line 964 + 0x16 bytes    C
      tcl87tg.dll!Tcl_ServiceEvent(int flags=-3)  Line 670 + 0xd
bytes    C
      tcl87tg.dll!Tcl_DoOneEvent(int flags=-3)  Line 967 + 0x9 bytes    C
      tk87tg.dll!Tk_MainLoop()  Line 2148 + 0x11 bytes    C
      tcl87tg.dll!Tcl_MainExW(int argc=-1, unsigned short * *
argv=0x02831bec, int (Tcl_Interp *)* appInitProc=0x004010a0,
Tcl_Interp * interp=0x00695bd0)  Line 580 + 0x5 bytes    C
      tclsh87tg.exe!wmain(int argc=1, unsigned short * *
argv=0x02831be8)  Line 129 + 0x25 bytes    C
      tclsh87tg.exe!__tmainCRTStartup()  Line 579 + 0x19 bytes    C
      tclsh87tg.exe!wmainCRTStartup()  Line 399    C


So the problem is triggered by execution of the line:

     ckfree(srcBlock.pixelPtr);

in tkImgListFormat.c:573


Do you want me to open a ticket regarding this?

Cheers,
Francois


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|

Re: TIP 166: Reference implementation

Simon Bachmann
In reply to this post by Francois Vogel
Am 26.03.2017 um 23:27 schrieb Francois Vogel:

> On OS X, running the test suite for the img*.test files triggers only
> one failure:
>
> ==== imgListFormat-6.19 ParseColor: invalid color: not a hex digit FAILED
> ==== Contents of test case:
>
>       photo1 put {#ABCDEF@.99 #ABCDEG@.99}
>
> ---- Test completed normally; Return code was: 0
> ---- Return code should have been one of: 1
> ==== imgListFormat-6.19 FAILED
>
> This is repeatable each time on OS X Sierra 10.12.3
Looks like 'G' is a valid hex digit on OS X... :-)

I'll take a closer look at this one of the next days.

Simon


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
123