Skip to content
This repository has been archived by the owner on Apr 19, 2021. It is now read-only.

optimize performance of images. #826

Merged
merged 1 commit into from
Jan 18, 2021
Merged

Conversation

nisarhassan12
Copy link
Contributor

@nisarhassan12
Copy link
Contributor Author

@ChristinFrohne I have swapped the hero image to use the gatsby-image component. The way Gatsby image works is that initially it displays a small blurred version of the original image while the original image is loading it is kind of same as it is on https://medium.com. What do you think about this effect? i.e currently on production users on slow internet connections can clearly see the glitchy loading of the screenshot while this effect takes care of that quite nicely.

@chrifro
Copy link
Contributor

chrifro commented Nov 3, 2020

@nisarhassan12 Do I understand it correctly that the blur effect only takes place if the Internet is slow? Then it's nice! But the effect shouldn't delay the loading process or be visible if there is no glitch.

@nisarhassan12
Copy link
Contributor Author

@ChristinFrohne I think that is the case. Do you see the blur on your side? I guess you are on a fast internet connection. On my side, I do see the blur instead of the glitch which to me looks a lot nicer.

@chrifro
Copy link
Contributor

chrifro commented Nov 3, 2020

Yes, I can see blur. If I'm comparing it with the current loading speed, it takes a bit longer with the blur. (See recording). Would be great if it's only visible on a slow connection. If the connection is fast, the images should load immediately without blur or glitching.
Bildschirmvideo aufnehmen 2020-11-03 um 15.59.37.zip

@nisarhassan12
Copy link
Contributor Author

@ChristinFrohne Not sure if that is possible. I have asked in Gatsby Discord let's wait for the response there.

If the connection is fast, the images should load immediately without blur or glitching.

I think it would be nice if we're to achieve the above even if we are unable to do so the blur is a lot better then the glitch for all users in general.

The blur effect does not look bad either. Yeah, there is a tradeoff the users on fast connections get to see the image instantly without the blur but generally, for all the users the blur is a lot better.

Do you like the animation when it changes from blur to unblur ?

Also I remember that last month on our call you were not able to see the blur on https://nisar.dev/#testimonials

@chrifro
Copy link
Contributor

chrifro commented Nov 9, 2020

To be clear: The blur effect is great, just be sure that it doesn't slow down the loading speed.
In the example it works well for me. 👍

It's a great idea. Please implement it :)

@nisarhassan12 nisarhassan12 force-pushed the nh/home-use-gatsby-image branch 2 times, most recently from 75eb043 to 7575677 Compare November 9, 2020 17:15
@nisarhassan12
Copy link
Contributor Author

nisarhassan12 commented Nov 9, 2020

@ChristinFrohne Should I use the gatsby-image component for all images for the sake of consistency or just a subset of them? Thanks.

@chrifro
Copy link
Contributor

chrifro commented Nov 10, 2020

Let's use it whenever there is a glitch. For me that's only at the top of the pages. Can you confirm that?

So then it would be great to have for the pricing icons on /pricing, the ice-cream icon on /features, the cloud-icon on /self-hosted, etc.

@nisarhassan12
Copy link
Contributor Author

nisarhassan12 commented Nov 10, 2020

Let's use it whenever there is a glitch. For me that's only at the top of the pages. Can you confirm that?

@ChristinFrohne Thanks, Sounds good.

So then it would be great to have for the pricing icons on /pricing, the ice-cream icon on /features, the cloud-icon on /self-hosted, etc.

Thanks. Yeah, the glitch is usually observable on the start screen of most of the pages so it would make sense to use gatsby-image for them.

Unfortunately we can't use SVGs with the gatsby-image component i.e in order for the glitch to be removed the image has to be a png or jpg.

Please provide me with the PNGs of the Ice Stick that appears on /features and for the images on start screens of pages like /github-student-developer-pack, github-teacher-toolbox/, /vendors, /enterprise, education and so on.

Also the goal for this PR is not just to remove the glitches but to make image performance better i.e improving the page speed test results and getting rid of the problems shown in the screenshot here https://github.com/gitpod-io/website/issues/821#issuecomment-717387737.

For now it would be ok to just work on the images with the glitch but at some point in near future, at least all of the large size images apart from small svg icons should be migrated to use gatsby-image if we are really willing to invest in making the overall performance of the site better.

@chrifro
Copy link
Contributor

chrifro commented Nov 11, 2020

Can we not just put the gatsby-image component on the galaxy background image?
Then we don't need to adjust all images. Also, it took quite some time adjusting the .svg's with the galaxy background, we shouldn't discard that work if we are not confident about the change. Would be great to hear @jankeromnes' or @gtsiolis' opinion about this :)

@nisarhassan12
Copy link
Contributor Author

Can we not just put the gatsby-image component on the galaxy background image?

I don't think it is possible the way gatsby-image works (Or even hacking our way around won't be a simple process and would take a lot of time and effort).

The glitch happens not becuase of the SVGs most of them are small and suitable in size the cause is I think the galaxy background that loads once the browser starts rendering the SVG it then loads the background and not with the SVG itself.

Also for every icon/image that uses the Galaxy background the same image is downloaded which is not good. Would be nice if we were to figure out a way via which we only download it once and not as many times it's used. I think this was what we were aiming for when we first implement the images with the Galaxy background.

The way it currently is the Galaxy background causes more problems then it solves.

image

@jbicker What do you think ? Thanks

@jankeromnes
Copy link
Contributor

Hi @nisarhassan12! So, as discussed in our 1:1:

  • Gatsby-Image is great for very large images (e.g. screenshots, or anything that Lighthouse singles out as being to big) because it will indeed send smaller images to smaller screens. (I personally find the blurry-first thing less interesting, but I'm okay with having it since it doesn't seem to slow down the website & maybe it's good to have some layout stability regardless of connection speed)

  • However, maybe don't use Gatsby-Image on smaller images, as it doesn't look so good: https://youtu.be/Me-NsTdvib8 (i.e. the rocket image should not use Gatsby-Image)

  • Also, regarding all the SVGs that use the galaxy background, I like your idea of turning them all into appropriately-sized PNGs (we generally want SVGs wherever possible, because they are lightweight & look crisp on almost any screen, however since we do want to use raster images in these (the galaxy background) I agree that this defeats the point of SVGs and we might as well use PNGs for those). So, please convert the small galaxy-themed images to PNGs (should still look crisp on high-definition screens while being as lightweight as possible)

@gtsiolis
Copy link
Contributor

FWIW, I agree with @jankeromnes in most points in https://github.com/gitpod-io/website/pull/826#issuecomment-728847337.

Most small images should be fine with leaving them as PNGs.

In addition we could convert all bitmap images to progressive JPEGs which will allow progressive loading for images but don't think we would benefit much from this in terms of performance for small images.

See JPEG compression and Interlacing:

There is also an interlaced progressive JPEG format, in which data is compressed in multiple passes of progressively higher detail.

@nisarhassan12
Copy link
Contributor Author

@ChristinFrohne I think it would be better if you were to provide PNGs for all of the SVG images that use the Galaxy background. If you don't have time then providing at least a subset of them which are critical i.e the ones on the start screen would be quite helpful for now and the remaining ones can be deferred for now but we must address them later at some point.

I had a little chat with @jbicker and we both concluded that we don't know of a way we can cache the Galaxy background to avoid it being loaded separately for every instance that uses it.

@chrifro
Copy link
Contributor

chrifro commented Nov 20, 2020

Okay. Thanks for digging into it @nisarhassan12

Here are all galaxy .png images. I hope the sizes fit well, let me know if I shall adjust something.
galaxy-icons-png.zip

@nisarhassan12 nisarhassan12 force-pushed the nh/home-use-gatsby-image branch 5 times, most recently from 91ef969 to 34f5e4e Compare November 23, 2020 18:48
@nisarhassan12
Copy link
Contributor Author

However, maybe don't use Gatsby-Image on smaller images, as it doesn't look so good: https://youtu.be/Me-NsTdvib8 (i.e. the rocket image should not use Gatsby-Image)

@jankeromnes I agree that it does not look so good but it does helps with the layout stability and better lighthouse score i.e using just the PNG without the gatsby-image has this layout re-reflow issue which does not look good either.

Maybe we should use the gatsby-image component in places where the layout glitch happens that is on the start of the screen. I tried using a PNG for the /self-hosted page and I noticed the glitch when the page loads and with the gatsby-image component It was quite smooth and there was no glitch.

Peek 2020-11-24 00-18

I think it's maybe ok to not use it for off the screen images i.e the Rocket image for which you shared the screencast not too sure My worry is user directly landing to a fragment like https://www.gitpod.io/features/#intelligence would be able to notice the glitch/layout re-flow if some image at the top is rendered with a different height once loaded.

We could put custom width and height attributes on the images to avoid the above but that would only work for a specific screen size and not universally across devices with quite different sizes.

What do you say? Thanks

@nisarhassan12
Copy link
Contributor Author

nisarhassan12 commented Nov 24, 2020

@jankeromnes This is how the tracedSVG looks also we can change the SVG fill to whatever colour we like:

Peek 2020-11-24 20-01

You can preview the change here which will be shortly deployed here https://deploy-preview-826--gitpod-website.netlify.app/self-hosted Also the tracedSVG is almost un-noticeable on fast internet connections.

@chrifro
Copy link
Contributor

chrifro commented Nov 27, 2020

The traced SVGs look a lot better. But I noticed that this image is not displayed correctly before it's filled. It's the same problem with the cloud. As setting up the blur seems to take a bit more effort, could you implement the changes first without the blur? Then we can also see how these changes influence the performance and decide if the blur is still necessary. Thanks!
Bildschirmfoto 2020-11-27 um 11 26 58

@chrifro chrifro mentioned this pull request Nov 27, 2020
6 tasks
@nisarhassan12
Copy link
Contributor Author

Thanks. @ChristinFrohne for the feedback I'll first implement these without the blur.

@nisarhassan12 nisarhassan12 force-pushed the nh/home-use-gatsby-image branch 2 times, most recently from 691d82d to 593503e Compare December 23, 2020 17:51
@nisarhassan12 nisarhassan12 marked this pull request as ready for review December 23, 2020 17:51
@nisarhassan12
Copy link
Contributor Author

@ChristinFrohne when you are back at work please provide me with the following icons he bag and the owl:

image

image

@nisarhassan12 nisarhassan12 force-pushed the nh/home-use-gatsby-image branch 3 times, most recently from a7177a8 to 2c941ea Compare December 23, 2020 19:10
@nisarhassan12
Copy link
Contributor Author

I have migrated most of the images on the most important pages i.e /, /features, /pricing and a few others. I think we should now get this PR merged and migrate the other images to use gatsby-image in a later PR.

@chrifro
Copy link
Contributor

chrifro commented Dec 28, 2020

In the preview there is still a blur. Could you improve the images first without adding the blur? We can still iterate on it afterwards.

Here are the missing images:
Student-pack

teacher-github

@nisarhassan12 nisarhassan12 force-pushed the nh/home-use-gatsby-image branch 2 times, most recently from 9b4d5c5 to 41a16d3 Compare December 29, 2020 17:24
@nisarhassan12
Copy link
Contributor Author

nisarhassan12 commented Dec 30, 2020

Thanks, @ChristinFrohne for providing the remaining images.

In the preview there is still a blur. Could you improve the images first without adding the blur? We can still iterate on it afterwards.

Yeah, that is only for the GItpod Screenshot on the home page and few other images maybe. I tried the tracedSVG instead of the blur like it is the case for all the other icons but the tracedSVG has some layout instability (Note: this is not the fault of Gatsby Image instead my inability to come up with a correct solution due to my lack of knowledge and understanding) with the blur the layout looks stable. see the below video where I slowed mimicked a slow internet connection so that this can be visualized better.

Peek.2020-12-30.10-27.mp4

Unless someone is able to figure out how the Layout could be stable with the tracedSVG I would say let's go with the blur? I think the blur is better than the layout instability What do you say? Thanks

Edit

I have removed the blur you can test the behaviour without it via the deploy preview. Also, I have migrated the icons you just provided to GatsbyImage as well.

@nisarhassan12 nisarhassan12 force-pushed the nh/home-use-gatsby-image branch 3 times, most recently from 7062da8 to 6b829fb Compare December 30, 2020 06:20
@chrifro
Copy link
Contributor

chrifro commented Jan 4, 2021

I feel like here is a misunderstanding. Why is there a dark image with white spaces before the original image loads?

I thought you wanted to swap all .svg images that contain a galaxy image with a .png to improve page speed.
Loading the dark images with the white spaces looks very weird to me. I'm sorry if I was unclear before or mixed something up.

I just checked the page speed again and according to it the galaxy images are not an issue. It suggests to not use .png formats and to simply decrease image size for several screenshots and Twitter profile pictures. Also, the page speed for https://deploy-preview-826--gitpod-website.netlify.app/ is slower than for https://developers.google.com/speed/pagespeed/insights/?hl=de&url=www.gitpod.io. I feel like we lost a bit of focus here.

From the linked issue, the PR is supposed to resize images properly. According to "page speed" this is the list of the images that should be resized.
Bildschirmfoto 2021-01-04 um 12 09 45

What do you think? Would it be best to start a new PR for that?

Sorry about the confusion, please clarify in case I missed something.

@nisarhassan12
Copy link
Contributor Author

Thanks, @ChristinFrohne. Sorry that I was not descriptive enough please let me know If I was not able to explain anything from what is said below.

Why is there a dark image with white spaces before the original image loads?

The way gatsby-image works is that while the original image is downloading it displays a placeholder image which is significantly smaller in size & loads really fast and once the original image has finished downloading it swaps the placeholder with the real image with a nice smooth animation.

The placeholder can be either a blurred smaller version of the original image or a traced SVG(which is a dark image often with whitespaces or what is shown in https://github.com/gitpod-io/website/pull/826#issuecomment-733032577).

While using a tracedSVG we can use any colour not just darker ones.

I just checked the page speed again and according to it, the galaxy images are not an issue. It suggests to not use .png formats and to simply decrease image size for several screenshots and Twitter profile pictures. Also, the page speed for https://deploy-preview-826--gitpod-website.netlify.app/ is slower than for https://developers.google.com/speed/pagespeed/insights/?hl=de&url=www.gitpod.io. I feel like we lost a bit of focus here.

Image performance is quite hard to get right and it has many dimensions and it is even harder without gatsby-image(because it automates a lot of stuff that otherwise has to be done manually by ourselves).

Let say we have a png image with size 800KB both with and without gatsby-image it would take the same amount of time to be downloaded depending on the internet speed but gatsby-image shines in the following areas as compared to using raw PNGs or JPEGs:

  • Instead of showing nothing it almost instantly shows a placeholder while the original image is being downloaded which makes the overall loading experience appear much nicer i.e it helps avoid parts of the layout jumping around due to re-flows.
  • If a user visits the site from a smaller device then gatsby-image takes care of automatically shipping a smaller version of the image that is aptly size in accordance with the device.
  • It automatically compresses/optimizes images with some image optimizer I guess it uses sharp.
  • It lazy loads the images which mean that not all the images on the page are downloaded initially i.e only the on-screen images are downloaded initially and images that are off-screen or down below in page only download when the user scrolls to the areas that contain them.

All of the areas where gatsby-image shines can be handled without the use of it but it would take a lot of time, effort and maintenance.

Performance Score

Regarding the page speed Are you sure? I just checked it now found out that without gatsby-image the score is 34 and with it, it's 61(almost the double):

Without gatsby-image

https://developers.google.com/speed/pagespeed/insights/?url=www.gitpod.io

image

Using Gatsby Image

https://developers.google.com/speed/pagespeed/insights/?url=https%3A%2F%2Fdeploy-preview-826--gitpod-website.netlify.app%2F

image

Regarding Properly Sizing Images

From the linked issue, the PR is supposed to resize images properly. According to "page speed" this is the list of the images that should be resized.

Sorry that I have not made it clear that the Twitter profiles are not using gatsby-image maybe they should as well? Also If we compare the below screenshot with what you shared for https://www.gitpod.io we can see that only one image that uses gatsby-image has this problem and that is the Gitpod screenshot. I have intentionally set the quality for the screenshot to be 100 I'm not sure but decreasing it may fix the problem with the screenshot as well.

image

Loading the dark images with the white spaces looks very weird to me.

Maybe let's go with the blur effect it is quite common among popular sites i.e medium.com uses something similar to gatsby-image and it uses the blurred image as the placeholder.

FYI, www.gitpod.io/docs already uses gatsby-image with the blur effect which uses blurred images as a placeholder.

Also maybe let's ask others what they think about the traced SVGs. I agree it does not look good for the Screenshot but for small icons and images, it's maybe ok to use?

@chrifro
Copy link
Contributor

chrifro commented Jan 11, 2021

  • The blur effects works nicely on docs. Let's use that for screenshots and Twitter profile pictures as well. (the tracing doesn't work well for screenshots and squared pictures as it creates holes)

  • The blur effect didn't work well on icons (https://github.com/gitpod-io/website/pull/826#issuecomment-732372082). For the icons the tracing effect that you are using now works nicely (e.g. pricing). Let's keep it

  • can you add the blur effect to the blog post images as well?

  • the desktop page speed got a lot faster for www.gitpod.io after merging the other PRs (current score 92), the netlify preview score is 87. So please keep the .svg format for the galaxy-background-icons for now, as it doesn't seem to be an issue. Otherwise, we'll just address it in another PR

Then we should also be able to merge and close this PR soon. Does that work for you? Thanks!

@nisarhassan12 nisarhassan12 force-pushed the nh/home-use-gatsby-image branch 4 times, most recently from 8cee5a2 to 2611fc7 Compare January 18, 2021 12:22
@nisarhassan12
Copy link
Contributor Author

nisarhassan12 commented Jan 18, 2021

  • can you add the blur effect to the blog post images as well?

I have added the blur effect to the post preivew for each post rendered on /blog and also on the screenshot rendered at the top of each individual post but for some reason it does not work for the images in body of the post so I have opened an issue for that https://github.com/gitpod-io/website/issues/949 which we will probably tackle later.

@nisarhassan12
Copy link
Contributor Author

nisarhassan12 commented Jan 18, 2021

Merging this tested a few pages it seems to give better overall results on Page Speed Insights.

@nisarhassan12 nisarhassan12 merged commit ba0cacd into master Jan 18, 2021
@nisarhassan12 nisarhassan12 deleted the nh/home-use-gatsby-image branch January 18, 2021 13:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants