support fractional blend percentages in nsBlender

VERIFIED FIXED in mozilla0.9

Status

P1
normal
VERIFIED FIXED
18 years ago
10 years ago

People

(Reporter: Chris244, Assigned: roc)

Tracking

Trunk
mozilla0.9
x86
All

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

18 years ago
See bug 33589 for a discussion of the issue and a test case.

nsBlender is used to implement the CSS opacity property.

This is an RFE to blend more accurately.  33589 was marked INVALID because it 
works as designed.  Hopefully we can get a better implementation at some point.  
The changes are trivial, but my previous solution was rejected.  This is a 5 
minute bug.

Comment 1

18 years ago
setting bug status to new.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reassigning bug to dcone
Assignee: kmcclusk → dcone

Comment 3

18 years ago
I will mark this as future because I don't think this will stop us from 
shipping, not as remind because I want to understand more about what it is being 
asked and how to best accomplish this.  
Status: NEW → ASSIGNED
Target Milestone: --- → Future
(Reporter)

Comment 4

18 years ago
Given two input pixels (source and destination) of fixed color I want to 
maximize the number of "in between" colors that can be generated by changing the 
opacity of the source pixel.  

--------
Example:
The destination pixel is white
The source pixel is black
We are working on a RGB device with 8-bits per color channel

By changing the floating point opacity property of the source pixel in small 
increments, we should be able to produce all the gray scale colors that our 
device supports (256 in this case).
--------

As it is now, only 101 gray values can be generated in the example scenario.
Robert: You rewrote the blender, right? Care to add fractional blending to it?
;-)
My changes, which I will attach, fix this issue and a lot of others.

The main purposes of my changes are to correctly handle the case where content 
was painted into a buffer pair with partial alpha values, and to speed the code 
up a lot. I also took the opportunity to clean up the code by removing unused 
variables, breaking out common cases into separate, faster functions, and adding 
a lot of comments.

I did make one interface change: the red/blue buffer stuff works much better and 
faster if the buffers are white/black instead. Otherwise, the partial alpha 
handling is very difficult and slow. So I removed the "background color" 
parameters and assume black and white. This requires some simple changes to the 
view manager.

I did not change the 8-bit blending code. It does not support source buffer 
pairs at all, anyway.

The existing code has other sources of inaccuracy other than the one Chris 
spotted. For example, for speed, it approximates division by 255 using an 8-bit 
right shift (dividing by 256). This leads to destination pixels somtimes being 
off by one from their true value. My changes avoid this problem by changing the 
opacity multiplier from out-of-255 to be out-of-256, so the right shift gives 
the correct result. [I still need to divide by 255 when the original content 
pixel was painted with partial alpha, but I worked out a fast approximation 
using an add and two shifts that gives exactly the right result for the range of 
inputs that are possible (0 to 255^2).]
Reassigning to Robert and praying this gets switched on by default soon.
Keywords: mozilla0.9, nsbeta1
Ian, you didn't reassign this to me. Did you mean to?
Oops. Yep indeedie I did. Sorry! :-)
Assignee: dcone → roc+moz
Status: ASSIGNED → NEW
I will attach a revised patch for my new blender. It's faster, more correct,
cleaner code, and better documented than the old blender. It fixes blending of
translucent conent, fixes roundoff errors and fixes this bug. Please review it
so I can check it in. Pavlov, please tell me who is supposed to review this.
Status: NEW → ASSIGNED
Priority: P3 → P2

Comment 14

18 years ago
Looks good.  Two concerns:

  * Bit weighting for 16bpp is assumed to be 555 on MacOS and 565 elsewhere.
    Is this safe?  The old nsBlender had the same assumption, so we're not
    regressing even if it is wrong.

  * Since NS_ASSERTION is a no-op when compiling non-debug, the 
    nsBlender::Blend() method needs more protection when the 
    conditions fail.
> Bit weighting for 16bpp is assumed to be 555 on MacOS and 565 elsewhere.
> Is this safe?

You answered this one yourself --- it's as good as what was already there. If 
it's not right, file another bug.

> Since NS_ASSERTION is a no-op when compiling non-debug, the
> nsBlender::Blend() method needs more protection when the
> conditions fail.

OK. New patch coming up.

Comment 17

18 years ago
Shouldn't there be some early exit in nsBlender::Blend() when the bitmap
formats are mismatched?
Yeah, there should. I'll put that in too.

Comment 19

18 years ago
roc: are you going to put the final touches on this patch so it can be
included in 0.9?
I'm targeting this at Mozilla 1.0 since I think it's not needed for Mozilla 0.9.
But the patch is ready for review and checkin ... so, whaddaya say, Tim and pavlov?

(The latest version just adds extra defensive code in case those assertions
fire, as requested by Tim)
Target Milestone: Future → mozilla1.0

Comment 22

18 years ago
Great.

[s]r=tor
Stuart, are you an appropriate reviewer for this?
tor, can you recommend a reviewer for this patch?
sr=blizzard

Looks fine from a style standpoint and much simpler than the old code.  And it's
(gasp!) well commented.
Thanks! I'll try to get it in this afternoon.
Priority: P2 → P1
Target Milestone: mozilla1.0 → mozilla0.9
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 28

18 years ago
Marking verified per last comments
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.