sb / smallbasic-editor

Home to the Small Basic editor (beta)
https://smallbasic-publicwebsite-code.azurewebsites.net/
MIT License
101 stars 34 forks source link

GraphicsWindow.DrawEllipse fills the ellipse (Edge Browser) #44

Closed nonkit closed 5 years ago

nonkit commented 5 years ago

GraphicsWindow.DrawEllipse operation fills the ellipse with "Black" instead of "Transparent". Sample code to reproduce is: GraphicsWindow.DrawEllipse(10, 10, 100, 100)

OmarTawfik commented 5 years ago

I tested on master, the following code:

GraphicsWindow.DrawEllipse(10, 10, 100, 100)
GraphicsWindow.DrawEllipse(20, 20, 100, 100)

Draws this as expected:

image

Please reopen if you think the output should be different.

nonkit commented 5 years ago

GraphicsWindow.DrawEllipse(10, 10, 100, 100)

still shows

issue 44 ellipse

Build environment may be something wrong. I'm checking now.

OmarTawfik commented 5 years ago

@nonkit does it still repro on http://superbasic-v2.azurewebsites.net?

nonkit commented 5 years ago

@OmarTawfik, yes. But I found this issue is alive only with Edge browser. Doesn't happen with Chrome, Firefox nor Safari. And also DrawRectangle and DrawTriangle are filled with black in Edge.

OmarTawfik commented 5 years ago

Another discrepancy with Edge's SVG engine 😢 I will reopen!

alxnull commented 5 years ago

https://github.com/sb/smallbasic-editor/blob/9f47c13efeefc0d6b7bd00faf9fff8487d56209c/Source/SmallBasic.Editor/Libraries/Utilities/PredefinedColors.cs#L14

Edge doesn't seem to understand #00000000 as being transparent. Interestingly, changing the value to transparent does seem to work for me even in Edge. Maybe this would be a solution?

OmarTawfik commented 5 years ago

@alxnull interesting! Do we use the alpha component of colors anywhere else in the libraries? this might impact it as well. For example, what happens if we set a shape color to #10101055? is it "half transparent"?

alxnull commented 5 years ago

@OmarTawfik The current version of SBO doesn't allow settings hex colors with eight digits, so the alpha component probably (?) isn't used anywhere yet. But if the hex code for BrushColor and PenColor would be allowed to have 8 digits in the style of SBO 0.91, we would get the following situation (this maybe should have been in a separate issue):

colors

All of this rectangles show a color with hex code #88FF1088, from left to right in SBO 1.0 on Firefox and SBO 0.91 (green color), SB Desktop (pink color) and SBO 1.0 on Edge (black). Short explanation:

(also related to #57)

OmarTawfik commented 5 years ago

Thanks for investigating! How about for this issue, let's use transparent as you suggested? For #57, we need to decide how we're going to support alpha colors in strings. I wasn't expecting the WPF vs CSS discrepancy ☹️

nonkit commented 5 years ago

I guess that CSS doesn't support #ffffffff but rgba(255, 255, 255, 1.0). SVG also doesn't support #ffffffff but opacity(1.0). But .NET supports #ffffffff (ARGB color). So, my suggestion is that SBO supports .NET style.

OmarTawfik commented 5 years ago

@nonkit @alxnull ultimately, I think we should be supporting everything the desktop version has. But if we do that, from the comment above, we would be breaking on Edge, because it doesn't support the alpha component. cc @EdPrice-MSFT any thoughts on that?

OmarTawfik commented 5 years ago

@alxnull also, if you tested transparent to work in all browsers, do you want to submit a fix to that? Otherwise, I'll look into it later this weekend.

alxnull commented 5 years ago

@OmarTawfik @EdPrice-MSFT If Edge support is important, there still would be the option to use color codes of the form rgba(255, 0, 0, 0.5) instead of hex codes (by converting the hex codes in SB code to rgba form internally) as they seem to be supported by all modern browsers (including Edge).

OmarTawfik commented 5 years ago

@alxnull this would be a large refactoring to make sure nothing is missed from the codebase. Let's decide on #57 first before doing that work.