Closed Bug 587106 Opened 14 years ago Closed 14 years ago

canvas strokeStyle getter mishandles opacity values smaller than 0.098

Categories

(Core :: Graphics: Canvas2D, defect, P1)

Product:
Core
Component:
Graphics: Canvas2D
Platform:
x86
Windows Vista
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla2.0b7

People

(Reporter: n8jedoor, Assigned: bzbarsky)

References

Details

(Keywords: testcase)

Attachments

(2 files)

test html page / script
327 bytes, text/html
Details
Fix
4.55 KB, patch
vlad
: review+
vlad
: approval2.0+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2.8) Gecko/20100722 Firefox/3.6.8 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2.8) Gecko/20100722 Firefox/3.6.8 (.NET CLR 3.5.30729)

In the canvas element, when setting a strokeStyle with opacity (alpha-value) lower than 0.1, is interpreted incorrectly. For example 0.0666 becomes 0.666, or 0.09 becomes 0.9
This messes up smooth fade-in effects.


Reproducible: Always

Steps to Reproduce:
- Run this script:

<html>
<head>
<script type="text/javascript">
function go(){
  var ctx=document.getElementById('canv').getContext('2d');
  ctx.strokeStyle = 'rgba(0,0,0, 0.09)';
  alert(ctx.strokeStyle);  //FireFox output: (about) 0.9, instead of 0.09
}
</script>
</head><body onload="go()">
<canvas id=canv></canvas>
</body></html>

Actual Results:  
Alert window says "rgba(0, 0, 0, 0.9016)".

Expected Results:  
"rgba(0, 0, 0, 0.09)"  (An alpha-value 10 times smaller).

Opera, Chrome and Safari handle this correctly.

    
    

    
  
I can reproduce with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b6pre) Gecko/20100904 Firefox/4.0b6pre. Seems to be canvas-specific.
Status: UNCONFIRMED → NEW
Component: General → Canvas: 2D
Ever confirmed: true
Keywords: testcase
Product: Firefox → Core
QA Contact: general → canvas.2d
Version: unspecified → Trunk

    
    

    
  
This is a bug in the strokeStyle getter.  It does:

971         PRUint32 alpha = NS_GET_A(aColor) * 100000 / 255;
972         CopyUTF8toUTF16(nsPrintfCString(100, "rgba(%d, %d, %d, 0.%d)",
973                                         NS_GET_R(aColor),
974                                         NS_GET_G(aColor),
975                                         NS_GET_B(aColor),
976                                         alpha),
977                         aStr);

In particular, this will give the wrong answer for any alpha channel value less than or equal to 26 (which corresponds to a floating-point alpha of 0.098 or less).
Summary: canvas strokeStyle opacity 0.09 is interpreted as 0.9 → canvas strokeStyle getter mishandles opacity values smaller than 0.098

    
    

    
  
The right solution is probably to stop using an nsPrintfCString for this.  In fact, I wish we could just reuse the nsCSSValue stringification code here, but that uses "transparent" for NS_RGBA(0,0,0,), which we may not want here.  If we didn't have to do the #rrggbb thing for the alpha == 1 case, we could at least factor out the nsCSSValue code....

    
    

    
  
Why do we even -have- that code in there.. what's wrong with just a %f with a precision!

    
    

    
  
It's from bug 329593, too bad I didn't decide to fix it properly then:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/canvas/src/nsCanvasRenderingContext2D.cpp&rev=1.128#570

570 // "%0.5f" in nsPrintfCString would use the locale-specific
571 // decimal separator. That's why we have to do this:
Blocks: 329593

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Fix
        — 
          — Splinter Review
      

    


  
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED

        Attachment #472634 -
        Flags: review?(vladimir)

    
  
Priority: -- → P1

    
  
Whiteboard: [need review]

    
    

    
  
Comment on attachment 472634 [details] [diff] [review]
Fix

Looks great, thanks -- sorry for the slow review!

        Attachment #472634 -
        Flags: review?(vladimir)

        Attachment #472634 -
        Flags: review+

        Attachment #472634 -
        Flags: approval2.0+

    
  
Whiteboard: [need review] → [need landing]

    
    

    
  
Pushed http://hg.mozilla.org/mozilla-central/rev/bcff46a05021

And then bustage fixes:
  http://hg.mozilla.org/mozilla-central/rev/5eb5819ad5bd
  http://hg.mozilla.org/mozilla-central/rev/39db042d6737
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8

    
  
Whiteboard: [need landing]

    
  
Target Milestone: mozilla2.0b8 → mozilla2.0b7
Depends on: 629883

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: